The spec says the mesh is defined using <column, row> and not <row, column>. Update the implementation to match that. https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#vertexMesh-attribute
Created attachment 170027 [details] Patch
Comment on attachment 170027 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170027&action=review Looks good. > LayoutTests/css3/filters/resources/discard-bottom-vertices.vs:13 > + if (position.y > 0.0) Why not use the a_triangleCoord.y instead? > LayoutTests/css3/filters/resources/discard-bottom-vertices.vs:14 > + position = vec4(-0.5); nit: use 4 spaces for indentation. nit: You are also setting the 3rd and 4th values to -0.5. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:939 > - meshParameters->append(cssValuePool().createValue(customOperation->meshRows(), CSSPrimitiveValue::CSS_NUMBER)); > meshParameters->append(cssValuePool().createValue(customOperation->meshColumns(), CSSPrimitiveValue::CSS_NUMBER)); > + meshParameters->append(cssValuePool().createValue(customOperation->meshRows(), CSSPrimitiveValue::CSS_NUMBER)); Add a test for the computed style.
Created attachment 170203 [details] Patch
Created attachment 170226 [details] Patch Removed tab character from test.
Comment on attachment 170027 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170027&action=review >> LayoutTests/css3/filters/resources/discard-bottom-vertices.vs:13 >> + if (position.y > 0.0) > > Why not use the a_triangleCoord.y instead? After our in-person discussion, I've changed the test to create a checkboard pattern by coloring the mesh tiles using a_triangleCoord. If the checkboard tiles are squares, the test passes. If not, the test fails. >> LayoutTests/css3/filters/resources/discard-bottom-vertices.vs:14 >> + position = vec4(-0.5); > > nit: use 4 spaces for indentation. > nit: You are also setting the 3rd and 4th values to -0.5. nit 1: Thanks for catching that. My editor was misbehaving. nit 2: The new test doesn't need to do that anymore. >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:939 >> + meshParameters->append(cssValuePool().createValue(customOperation->meshRows(), CSSPrimitiveValue::CSS_NUMBER)); > > Add a test for the computed style. This is already covered by the computed style tests. In computed style, we can only check that the order we input the mesh attributes is the order we output them. For example, we can only check that the mesh coords of "10 20" come back as "10 20". They're not labeled, so I can't test which one represents numColumns vs. numRows. However, the test I've added verifies the numColumns vs. numRows difference by rendering it.
Thanks for the review, Dean!
Comment on attachment 170226 [details] Patch Clearing flags on attachment: 170226 Committed r132416: <http://trac.webkit.org/changeset/132416>
All reviewed patches have been landed. Closing bug.