Re: [webkit-dev] Testing feature suggestion: animation/interaction pixel-results on the fly

2013-02-14 Thread Dirk Pranke
On Wed, Feb 13, 2013 at 11:33 PM, Benjamin Poulain benja...@webkit.org wrote:
 On Wed, Feb 13, 2013 at 11:16 PM, Dirk Pranke dpra...@chromium.org wrote:

  Those changes are not harmless. There are people monitoring tests
  results
  full time in order to keep WebKit in good shape. No other part of WebKit
  require continuous attention.

 I'm sorry, but either I don't understand Dongsung's suggestion, or I
 don't understand your criticism. What does this sentence paragraph
 mean? Are you suggesting someone needs to look at this type of test
 full time in a way that we don't have to look at the other tests?


 My lack of sleep is likely to blame here :)

 All I am saying is tests have a hight cost when they start failing, we
 should be careful how we design them so we can easily triage bad tests,
 flaky tests, and real errors.


No argument here :).


  Case 1: CSS Filters  Shaders
  I wanted this test functionality when I commented
  http://webkit.org/b/97859#c19
  If I want to make gaussian blur test, I prefer using 'getPixel' test as
  follows,
 
 
  Why wasn't a ref-test a better solution in this particular case?
 

 I'm curious, what would you imagine the ref test contains?


 If I am not mistaken, the composition operations are parallel with the ones
 of SVG and Canvas (aren't they?).
 I would have attempted comparing the 3 implementations as it seems to me the
 pixels values should be the same.

 That was a genuine question about that case :)



  Case 2: Fixed Position Element
  [...]
  function repeatedlyCalledDuringScrolling() {
  ASSERT(getPixel(15, 9) == white);
  ASSERT(getPixel(15, 10) == green);
  ASSERT(getPixel(9, 15) == white);
  ASSERT(getPixel(10, 15) == green);
  
  }
 
 
  I think this shows what I said about correctness and readability:
  -Asserting the correctness of the test and the result becomes close to
  impossible for the reader. One has to review the full code to have a
  chance
  of understanding an error.
  -You cannot cover non trivial cases (images, text, form elements, etc).
  -And it is inefficient. You have to render each frame on the UIProcess,
  move
  it to the WebProcess, and box it for JavaScript to process (with pixel
  format conversions depending on your graphics system)
 
  Of the ideas raised, I think this is one of my least favorite for
  testing
  fixed positioning.
 

 Isn't his suggestion the equivalent of what we do today in text-only
 tests? i.e., printing pass or fail and making you have to look at
 the test itself to see what's being tested?

 If the correctness of the rendering depends on those 4 specific pixels
 having those four specific values, how exactly are you going to verify
 that by looking at it?

 Again, I think I'm just not understanding you here?


 When looking at a test test, you follow the flow to know what it is supposed
 to do and where it breaks.

 How are you supposed to know, _by reading the code_, that the color at
 position 15, 9 should be white?


is the code the test code, or the source code we're testing? Of
course, without any larger context, those assertions are impossible to
interpret. I figured he was just indicating what he meant by a
programmatic check, and a real test case would be more informative.

And of course it's easy to write bad tests that are neither readable
nor maintainable. But we also have lots of pixel tests that are
difficult for people not intimately familiar with the code being
tested to evaluate for correctness, too.

Lastly, I don't think he was suggesting that this would replace all
pixel tests; as you say, it is more well suited for some times of
tests than others.

As far as efficiency goes, even moving the picture back and forth
between two processes is still going to be far faster than running a
second reftest and comparing the output of the two pages.

-- Dirk
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Testing feature suggestion: animation/interaction pixel-results on the fly

2013-02-14 Thread Dongsung Huang


 I'm curious, what would you imagine the ref test contains?


 If I am not mistaken, the composition operations are parallel with the
 ones of SVG and Canvas (aren't they?).
 I would have attempted comparing the 3 implementations as it seems to me
 the pixels values should be the same.

 That was a genuine question about that case :)


Thank you for feedback! :)
I presented gaussian blur case to express my opinion effectively.

As I know, Adobe want to draw CSS Filters and SVG Filters using the same
code. And Canvas does not have something like Filters.
What I want to say is that there are some cases in which snapshot
functionality is more useful.


  Case 2: Fixed Position Element
  [...]
  function repeatedlyCalledDuringScrolling() {
  ASSERT(getPixel(15, 9) == white);
  ASSERT(getPixel(15, 10) == green);
  ASSERT(getPixel(9, 15) == white);
  ASSERT(getPixel(10, 15) == green);
  
  }
 
 
  I think this shows what I said about correctness and readability:
  -Asserting the correctness of the test and the result becomes close to
  impossible for the reader. One has to review the full code to have a
 chance
  of understanding an error.
  -You cannot cover non trivial cases (images, text, form elements, etc).
  -And it is inefficient. You have to render each frame on the UIProcess,
 move
  it to the WebProcess, and box it for JavaScript to process (with pixel
  format conversions depending on your graphics system)
 
  Of the ideas raised, I think this is one of my least favorite for
 testing
  fixed positioning.
 

 Isn't his suggestion the equivalent of what we do today in text-only
 tests? i.e., printing pass or fail and making you have to look at
 the test itself to see what's being tested?

 If the correctness of the rendering depends on those 4 specific pixels
 having those four specific values, how exactly are you going to verify
 that by looking at it?

 Again, I think I'm just not understanding you here?


 When looking at a test test, you follow the flow to know what it is
 supposed to do and where it breaks.

 How are you supposed to know, _by reading the code_, that the color at
 position 15, 9 should be white?

 Benjamin



I agree on Dirk's opinion. I supports 3 points as follows:
1. we can help to understand what's goal of test by comments
2. when we want to see the graphical result, we can use MiniBrowser like
other text based test.
3. philip's canvas tests (http://philip.html5.org/tests/canvas/suite/tests/)
already use well getImageData to test.

I agree that snapshot test can make hardly maintainable tests, but we have
review process and we can make succinct tests like philip canvas test.
And when pixel test is more suitable, we can use pixel test also.

Best regards,

Dongsung Huang
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] sln files with wrong line endings

2013-02-14 Thread Rafael Brandao
This also happened to me, quite annoying. :-(
I've followed the thread
https://groups.google.com/a/chromium.org/forum/?fromgroups=#!topic/chromium-dev/YpKL4xiJHPQ
and
did the following:
editing .gitattributes to disable the eol=crlf flag for .sln files, then
doing a reset/checkout, then un-editing .gitattributes.


On Thu, Feb 14, 2013 at 6:45 AM, John Yani van...@gmail.com wrote:

 Checkout this link:
 https://help.github.com/articles/dealing-with-line-endings

 I don't know what the webkit convention is when dealing with line-endings,
 but you may want to make sln files to always have crlf line-ending.

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 https://lists.webkit.org/mailman/listinfo/webkit-dev




-- 
Rafael Brandao @ INdT
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] sln files with wrong line endings

2013-02-14 Thread Darin Adler
Typically in Subversion the fix for a problem like this is to set the eol-style 
explicitly; presumably to CRLF for Windows project and solution files and to LF 
or native for plain old source files. Once that property is set, people 
changing file later on can’t cause problems with inconsistent line endings, 
because Subversion will check the line endings at commit time.

I have no idea how having git in the mix affects this. In fact, I don’t know 
how to set a Subversion property using git-svn which means I can’t easily just 
jump in and fix this like I would have in the past.

-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] sln files with wrong line endings

2013-02-14 Thread Adam Roben
I'm pretty sure r142864 fixed this (perhaps unintentionally) by
changing all line endings in DumpRenderTree.sln from CRLF to LF.

On Thu, Feb 14, 2013 at 11:11 AM, Darin Adler da...@apple.com wrote:
 Typically in Subversion the fix for a problem like this is to set the 
 eol-style explicitly; presumably to CRLF for Windows project and solution 
 files and to LF or native for plain old source files. Once that property is 
 set, people changing file later on can’t cause problems with inconsistent 
 line endings, because Subversion will check the line endings at commit time.

 I have no idea how having git in the mix affects this.

WebKit's .gitattributes file contains the following:

*.vcproj eol=crlf
*.vsprops eol=crlf
*.sln eol=crlf

This tells git, When storing this file in the repository, convert all
line endings to LF. When checking out this file to disk, convert all
line endings to CRLF.

If the committed version of the file in the repository contains CRLF
line endings, git will show a diff that converts the CRLF line endings
to LF. eol=crlf means that the line endings *in the repository* should
be LF, so git is trying to fix it.

I think the thing to do is to set svn:eol-style to native for all
.vcproj, .vsprops, and .sln files. That way Subversion will commit the
files using LF line endings (like git wants), and check them out using
CRLF on Windows (like Visual Studio wants).

 In fact, I don’t know how to set a Subversion property using git-svn which 
 means I can’t easily just jump in and fix this like I would have in the past.

I don't think git-svn can set properties. When I've had to change
properties in the past I've checked out the appropriate subset of
WebKit using Subversion and made my changes there.

-Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] sln files with wrong line endings

2013-02-14 Thread Osztrogonác Csaba

Hi All,

Here is an old bug report about it:
Files with CRLF lineendigs without svn:eol-style=native kills git svn 
repositories

https://bugs.webkit.org/show_bug.cgi?id=96934

The general fix would be to force all Windows project file have
svn:eol-style=native svn property. git-svn users who commit this
files can do it automatically inside: ~/.subversion/config
*.vsprops = svn:eol-style=native
*.vcproj = svn:eol-style=native

But we should force all developers to set it somehow.
There is a working solution for png files: 
https://trac.webkit.org/changeset/122311


It force commit svn:mime-type image/png svn property for SVN users
and warn GIT-SVN users if they didn't set their ~/.subversion/config
properly. I think a volunteer can do a similar thing for Windows
project files.

Ossy

Adam Roben írta:

I'm pretty sure r142864 fixed this (perhaps unintentionally) by
changing all line endings in DumpRenderTree.sln from CRLF to LF.

On Thu, Feb 14, 2013 at 11:11 AM, Darin Adler da...@apple.com wrote:

Typically in Subversion the fix for a problem like this is to set the eol-style 
explicitly; presumably to CRLF for Windows project and solution files and to LF 
or native for plain old source files. Once that property is set, people 
changing file later on can't cause problems with inconsistent line endings, 
because Subversion will check the line endings at commit time.

I have no idea how having git in the mix affects this.


WebKit's .gitattributes file contains the following:

*.vcproj eol=crlf
*.vsprops eol=crlf
*.sln eol=crlf

This tells git, When storing this file in the repository, convert all
line endings to LF. When checking out this file to disk, convert all
line endings to CRLF.

If the committed version of the file in the repository contains CRLF
line endings, git will show a diff that converts the CRLF line endings
to LF. eol=crlf means that the line endings *in the repository* should
be LF, so git is trying to fix it.

I think the thing to do is to set svn:eol-style to native for all
.vcproj, .vsprops, and .sln files. That way Subversion will commit the
files using LF line endings (like git wants), and check them out using
CRLF on Windows (like Visual Studio wants).


In fact, I don't know how to set a Subversion property using git-svn which 
means I can't easily just jump in and fix this like I would have in the past.


I don't think git-svn can set properties. When I've had to change
properties in the past I've checked out the appropriate subset of
WebKit using Subversion and made my changes there.

-Adam


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] Enable CANVAS_PATH by default

2013-02-14 Thread Dirk Schulze
Hi WebKit folks,

I worked on the Path interface defined by the Canvas spec of W3C and WHATWG 
[1][2] for the last couple of weeks.

Summary:
Canvas supports a new DOM interface called Path. The Path interface takes a 
series of very well known path methods like moveTo, lineTo, cubicCurveTo, rect 
and allows to create and keep a path independent of a Canvas context. 
Additionally, I added the attribute 'currentPath' to the Canvas context to 
provide read and write access to the current path created on the Canvas 
context. Code snippet:

var path = new Path();
path.rect(0,0,100,100);

var ctx = canvas.getContext('2d');
ctx.currentPath = path;
ctx.lineTo(200,200);
ctx.closePath();

var path2 = ctx.currentPath; // path2 != path

Not implemented are addText, addPath,  addPathByStrokingText. Another proposal 
from Rik Cabanier[3] seems to address the idea behind these methods better.

I would like to ask to enable CANVAS_PATH by default on trunk. Ports can 
opt-out the flag again. More information about some implementation details in a 
short article[4]. If there are any concerns, suggestions or questions, I am 
happy to answer them.

Greetings,
Dirk

[1] http://www.w3.org/html/wg/drafts/2dcontext/html5_canvas/#path-objects
[2] 
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#path-objects
[3] http://blogs.adobe.com/webplatform/2013/01/31/revised-canvas-paths/
[4] http://dschulze.com/blog/articles/10/html-canvas-path-object-in-webkit
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] Ports: Converting media controls to -webkit-flex

2013-02-14 Thread Christian Biesinger
Hi,

currently, media controls all use the deprecated flex box
(-webkit-box). In https://bugs.webkit.org/show_bug.cgi?id=109775, I am
converting them to use the new flex box (-webkit-flex).

I have verified that the result looks correct on the Chromium and Mac
ports, but I am unable to build/test the other ports. You may want to
double-check that the layout looks correct (if you have more than one
flexing control, the layout may be different, e.g. if you have both a
timeline and a volume slider. Otherwise there should be no difference)

-christian
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Enable CANVAS_PATH by default

2013-02-14 Thread James Robinson
On Thu, Feb 14, 2013 at 9:55 AM, Dirk Schulze dschu...@adobe.com wrote:

 Hi WebKit folks,

 I worked on the Path interface defined by the Canvas spec of W3C and
 WHATWG [1][2] for the last couple of weeks.

 Summary:
 Canvas supports a new DOM interface called Path. The Path interface takes
 a series of very well known path methods like moveTo, lineTo, cubicCurveTo,
 rect and allows to create and keep a path independent of a Canvas context.
 Additionally, I added the attribute 'currentPath' to the Canvas context to
 provide read and write access to the current path created on the Canvas
 context. Code snippet:

 var path = new Path();
 path.rect(0,0,100,100);

 var ctx = canvas.getContext('2d');
 ctx.currentPath = path;
 ctx.lineTo(200,200);
 ctx.closePath();

 var path2 = ctx.currentPath; // path2 != path

 Not implemented are addText, addPath,  addPathByStrokingText. Another
 proposal from Rik Cabanier[3] seems to address the idea behind these
 methods better.

 I would like to ask to enable CANVAS_PATH by default on trunk. Ports can
 opt-out the flag again. More information about some implementation details
 in a short article[4]. If there are any concerns, suggestions or questions,
 I am happy to answer them.


Could you please add a runtime enable flag before flipping this on for
chromium?  Thanks.

- James



 Greetings,
 Dirk

 [1] http://www.w3.org/html/wg/drafts/2dcontext/html5_canvas/#path-objects
 [2]
 http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#path-objects
 [3] http://blogs.adobe.com/webplatform/2013/01/31/revised-canvas-paths/
 [4] http://dschulze.com/blog/articles/10/html-canvas-path-object-in-webkit
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev