Bug 88786 – TextureMapperLayer: invisible layers do not let their children to be painted

• Component: New Bugs
• Status: RESOLVED FIXED

[Sergio Villar Senin]

[GTK] TextureMapperLayer: invisible layers do not let their children to be painted

[Sergio Villar Senin]

Created attachment 146866 [details]
Patch

[Sergio Villar Senin]

In the particular case of the bug mentioned in the ChangeLog there are 3 boxes, the first one (the bigger) has the second inside, and this one has another one inside (the smaller). The second box is rotated 180º and it becomes invisible because the backface is marked as invisible. Then the third box is rotated another 180º so it becomes visible but it was not painted because its father was declared as invisible and thus it was not rendering their children.

[Martin Robinson]

A link to a test case would be pretty useful here, I think.

[Noam Rosenthal]

Comment on attachment 146866 [details]
Patch

I understand the bug, but the fix is not right. We still want to exit early if the opacity is 0, the size is empty etc. We should add specific tests of culling, and only paint the children if we're preserving 3D.

[Sergio Villar Senin]

(In reply to comment #3)
> A link to a test case would be pretty useful here, I think.

This is the one I used for testing:

http://svn.webkit.org/repository/webkit/trunk/LayoutTests/compositing/backface-visibility/backface-visibility-3d.html

[Sergio Villar Senin]

Created attachment 147258 [details]
Patch

[Noam Rosenthal]

Comment on attachment 147258 [details]
Patch

This is not the right logic. It should be:
- opacity 0, empty size, or (backfacing and backface-visibility is false and we're not preserving 3D)? return early at paintRecursive
- (backfacing and backface-visibility is false) ? return early and paintSelf

[Sergio Villar Senin]

(In reply to comment #7)
> (From update of attachment 147258 [details])
> This is not the right logic. It should be:
> - opacity 0, empty size, or (backfacing and backface-visibility is false and we're not preserving 3D)? return early at paintRecursive



> - (backfacing and backface-visibility is false) ? return early and paintSelf

(In reply to comment #7)
> (From update of attachment 147258 [details])
> This is not the right logic. It should be:
> - opacity 0, empty size, or (backfacing and backface-visibility is false and we're not preserving 3D)? return early at paintRecursive

That preserves3D is interesting because I don't understand the expected results of the third test case of http://svn.webkit.org/repository/webkit/trunk/LayoutTests/compositing/backface-visibility/backface-visibility-non3d.html. Shouldn't the lime div be not visible?

[Sergio Villar Senin]

(In reply to comment #8)
> (In reply to comment #7)

> > (From update of attachment 147258 [details] [details])
> > This is not the right logic. It should be:
> > - opacity 0, empty size, or (backfacing and backface-visibility is false and we're not preserving 3D)? return early at paintRecursive
> 
> That preserves3D is interesting because I don't understand the expected results of the third test case of http://svn.webkit.org/repository/webkit/trunk/LayoutTests/compositing/backface-visibility/backface-visibility-non3d.html. Shouldn't the lime div be not visible?

Well that's is really not that important now so ignore it.

So please correct me if I'm wrong, but this is how I understand the issue:
- we cannot skip a whole hierarchy in paintRecursive() just because we're backfacing and not preserving 3D because even in that case we could have a visible child (including childs backfacing with backface-visibility: visible). See for example test #3 in LayoutTests/compositing/backface-visibility/backface-visibility-non3d.html. Not preserving 3d as I understand it, does not invalidate the CSS painting order.
- in paintSelf() we should basically early return if (!m_state.visible) (which already takes into account the backface and its visiblity)

Following this rationale I cooked a 3rd patch that makes both LayoutTests/compositing/backface-visibility/backface-visibility-non3d.html and LayoutTests/compositing/backface-visibility/backface-visibility-3d.html work fine. But before uploading it I'd like to know your opinion about what I said above.

[Sergio Villar Senin]

Created attachment 147748 [details]
Patch

Noam, I was talking about something like this

[Sergio Villar Senin]

Committed r120577: <http://trac.webkit.org/changeset/120577>