Bug 68087 – [chromium] CSS 3D drawing artifacts

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[David Reveman]

There seems to be some instability in the edge calculation code used for anti-aliasing. Artifacts are visible in this page:

http://css-class.com/test/css/3/transform-color-cube3.htm

I don't have a layout test top reproduce this yet. Working on it.

[David Reveman]

Created attachment 107648 [details]
Layout test

[David Reveman]

Created attachment 107689 [details]
Patch

[David Reveman]

Created attachment 107723 [details]
Patch

[Vangelis Kokkevis]

Comment on attachment 107723 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=107723&action=review

Looks good.  One possible suggestion for simplifying the shaders a bit. Also in the new 3d-corners test, the obtuse angle of the triangle looks a bit flattened.  Is that expected?

> Source/WebCore/platform/graphics/chromium/ShaderChromium.cpp:321
> +            gl_FragColor = texColor * alpha * min(min(a0, a2) * min(a1, a3), min(a4, a6) * min(a5, a7));

Would it make sense to pass in the edge info as two mat4's and replace the 8 dot products and clamps by 2 matrix-vector multiplies and 2 clamps?

[David Reveman]

Comment on attachment 107723 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=107723&action=review

>> Source/WebCore/platform/graphics/chromium/ShaderChromium.cpp:321
>> +            gl_FragColor = texColor * alpha * min(min(a0, a2) * min(a1, a3), min(a4, a6) * min(a5, a7));
> 
> Would it make sense to pass in the edge info as two mat4's and replace the 8 dot products and clamps by 2 matrix-vector multiplies and 2 clamps?

Not sure that would buy us anything. I think the clamps to [0.0-1.0] range are basically free on most GPUs. I guess it's possible that two mat4/vec4 multiplies are more efficient than 8 dot3 ops on some GPUs even though it theoretically requires more math. I would prefer to keep the current code as using mat4 would require more uniforms and make the intentions of the code a bit more obscure.

[David Reveman]

(In reply to comment #4)
> (From update of attachment 107723 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=107723&action=review
> 
> Looks good.  One possible suggestion for simplifying the shaders a bit. Also in the new 3d-corners test, the obtuse angle of the triangle looks a bit flattened.  Is that expected?

Sorry, I missed this question in my last reply. The alpha value at the end of these sharp corners is not perfect. Actually the only time this method produces alpha values that perfectly matches coverage-based anti-aliasing is for 90 degree angles. All other angles will be slightly off but usually close enough that it's not really noticeable. The corners in the 3d-corners test are pushing this to the extreme.

[Vangelis Kokkevis]

(In reply to comment #6)
> (In reply to comment #4)
> > (From update of attachment 107723 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=107723&action=review
> > 
> > Looks good.  One possible suggestion for simplifying the shaders a bit. Also in the new 3d-corners test, the obtuse angle of the triangle looks a bit flattened.  Is that expected?
> 
> Sorry, I missed this question in my last reply. The alpha value at the end of these sharp corners is not perfect. Actually the only time this method produces alpha values that perfectly matches coverage-based anti-aliasing is for 90 degree angles. All other angles will be slightly off but usually close enough that it's not really noticeable. The corners in the 3d-corners test are pushing this to the extreme.

Sounds good. You're probably right about the shader performance issue as well.  Unofficial r+ from me.

[James Robinson]

Comment on attachment 107723 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=107723&action=review

Then official R=me.  Fix the test and this is ready to land.

> LayoutTests/platform/chromium/compositing/3d-corners-expected.txt:3
> +layer at (0,0) size 800x856
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x600

you shouldn't have a render tree dump since it doesn't reveal anything interesting about this test. instead add this to the <script> of your test:

if (window.layoutTestController)
  layoutTestController.dumpAsText(true);

[David Reveman]

Created attachment 108492 [details]
Patch

[James Robinson]

Comment on attachment 108492 [details]
Patch

Close, you still need to add 3d-corners-expected.txt (even though it's just one newline) or the test will fail on the bots.

[David Reveman]

Created attachment 108495 [details]
Patch

[James Robinson]

Comment on attachment 108495 [details]
Patch

That's the ticket.

[WebKit Review Bot]

Comment on attachment 108495 [details]
Patch

Attachment 108495 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9822129

New failing tests:
svg/custom/svg-fonts-word-spacing.html

[WebKit Review Bot]

Comment on attachment 108495 [details]
Patch

Clearing flags on attachment: 108495

Committed r95870: <http://trac.webkit.org/changeset/95870>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.