Bug 106672 – [Texmap][GTK] The poster-circle doesn't appear.

• Component: WebKitGTK
• Status: RESOLVED FIXED

[ChangSeok Oh]

The poster-circle animation isn't shown before resizing the window.
Visit http://www.webkit.org/blog-files/3d-transforms/poster-circle.html
I checked this issue is still there to WebKit 1, 2 using texturemapper AC for gtk port.

[ChangSeok Oh]

Created attachment 182360 [details]
Patch

[Noam Rosenthal]

Comment on attachment 182360 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        No new tests. Covered by existing tests.

Which existing test covers it?
I think we should add a new test for this or the regression would come back.

[ChangSeok Oh]

Created attachment 182823 [details]
Patch

[WebKit Review Bot]

Attachment 182823 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/comp..." exit_code: 1
LayoutTests/compositing/visibility/visibility-composited-animation-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[ChangSeok Oh]

Comment on attachment 182360 [details]
Patch

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

>> Source/WebCore/ChangeLog:11
>> +        No new tests. Covered by existing tests.
> 
> Which existing test covers it?
> I think we should add a new test for this or the regression would come back.

Yeah.. I didn't realize there wasn't a proper test for this bug since current gtk port has disabled compositing tests..
Let me include a pixel test for this.

[ChangSeok Oh]

(In reply to comment #4)
> Attachment 182823 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/comp..." exit_code: 1
> LayoutTests/compositing/visibility/visibility-composited-animation-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
> Total errors found: 1 in 6 files

Maybe false alarm? It seems related with system environment setup.. :( 
Or is there any special way to include png files?

[WebKit Review Bot]

Comment on attachment 182823 [details]
Patch

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

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html

[ChangSeok Oh]

Created attachment 182995 [details]
Patch

[WebKit Review Bot]

Attachment 182995 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/comp..." exit_code: 1
LayoutTests/compositing/visibility/visibility-composited-animation-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Noam Rosenthal]

Comment on attachment 182995 [details]
Patch

I don't understand what this test does, and how it relates to the DrawsContent flag.

[ChangSeok Oh]

(In reply to comment #10)
> (From update of attachment 182995 [details])
> I don't understand what this test does, and how it relates to the DrawsContent flag.

If you run the attached test, you can see nothing drawn on webkit with texturemapper.
The rotating blue rectangle should be drawn on a separated GraphicsLayer since it has an animation property. Even though a GraphicsLayer is created for it and layer tree is structured successfully, the layer doesn't appear on screen. Because it doesn't have a change to call setNeedsDisplay for the layer. For these reason, we should call setNeedsDisplay for the layer(having animation) in proper somewhere.
As my investigation. mac port and gtk port with clutter ac work fine for this test. because I think they are call setNeedsDisplay when the drawContent value is changed.

The test verifies that the layer (rotating blue rectangle in the test case)  is actually drawn on window by comparing with the expected png result.

[ChangSeok Oh]

Comment on attachment 182995 [details]
Patch

Need a trivial tweak.

[ChangSeok Oh]

Created attachment 183008 [details]
Patch

[ChangSeok Oh]

@Noam
Any concern makes you hesitate to review this? :)

[Noam Rosenthal]

Comment on attachment 183008 [details]
Patch

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

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:391
> +    if (changeMask & DrawsContentChange && graphicsLayer->drawsContent())

Please add parantheses,
if ((changeMask & DrawsContentChange) && graphicsLayer->drawsContent())

[ChangSeok Oh]

Created attachment 183745 [details]
Patch

[ChangSeok Oh]

Comment on attachment 183008 [details]
Patch

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

>> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:391
>> +    if (changeMask & DrawsContentChange && graphicsLayer->drawsContent())
> 
> Please add parantheses,
> if ((changeMask & DrawsContentChange) && graphicsLayer->drawsContent())

Done. Thanks!

[WebKit Review Bot]

Attachment 183745 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/comp..." exit_code: 1
LayoutTests/compositing/visibility/visibility-composited-animation-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 183745 [details]
Patch

Clearing flags on attachment: 183745

Committed r140347: <http://trac.webkit.org/changeset/140347>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.