Bug 75910 - [Qt][Texmap] LayoutTests/compositing/masks/masked-ancestor does not render correctly.
Summary: [Qt][Texmap] LayoutTests/compositing/masks/masked-ancestor does not render co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Noam Rosenthal
URL:
Keywords: Qt
Depends on:
Blocks: 75918
  Show dependency treegraph
 
Reported: 2012-01-09 16:39 PST by Noam Rosenthal
Modified: 2012-01-11 11:50 PST (History)
4 users (show)

See Also:


Attachments
Patch (6.59 KB, patch)
2012-01-09 16:42 PST, Noam Rosenthal
hausmann: review+
hausmann: commit-queue-
Details | Formatted Diff | Diff
Patch (6.42 KB, patch)
2012-01-11 10:30 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Noam Rosenthal 2012-01-09 16:39:48 PST
Masks do not render correctly with intermediate surfaces.
Comment 1 Noam Rosenthal 2012-01-09 16:42:04 PST
Created attachment 121759 [details]
Patch
Comment 2 Simon Hausmann 2012-01-11 06:25:28 PST
Comment on attachment 121759 [details]
Patch

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

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:278
> +    int tiles = 0;
> +
> +#if USE(TILED_BACKING_STORE)
> +    if (m_state.tileOwnership == ExternallyManagedTiles) {
> +        HashMap<int, ExternallyManagedTile>::const_iterator end = m_externallyManagedTiles.end();
> +        for (HashMap<int, ExternallyManagedTile>::const_iterator it = m_externallyManagedTiles.begin(); it != end; ++it) {
> +            if (it->second.frontBuffer.texture) {
> +                if (++tiles > 1)
> +                    return true;
> +            }
> +        }
> +
> +        return false;
> +    }
> +#endif
> +
> +    for (size_t i = 0; i < m_ownedTiles.size(); ++i) {
> +        if (m_ownedTiles[i].texture) {
> +            if (++tiles > 1)
> +                return true;
> +        }
> +    }
> +
> +    return false;
> +}

Do you really need the tiles variable? :)

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:574
> +        TextureMapperPaintOptions optionsForMask(options);
> +        optionsForMask.isSurface = true;
> +        m_state.maskLayer->paintSelf(options);

Shouldn't the call to paintSelf pass optionsForMask instead of options?
Comment 3 Noam Rosenthal 2012-01-11 06:29:43 PST
> Do you really need the tiles variable? :)
Guess it could be a boolean, but I don't see what that would buy. We need to see if there is more than one tile with contents, so we have to maintain at one variable after we find the first tile with content.

> 
> > Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:574
> > +        TextureMapperPaintOptions optionsForMask(options);
> > +        optionsForMask.isSurface = true;
> > +        m_state.maskLayer->paintSelf(options);
> 
> Shouldn't the call to paintSelf pass optionsForMask instead of options?
Right, my oversight.
Comment 4 Simon Hausmann 2012-01-11 06:38:19 PST
(In reply to comment #3)
> > Do you really need the tiles variable? :)
> Guess it could be a boolean, but I don't see what that would buy. We need to see if there is more than one tile with contents, so we have to maintain at one variable after we find the first tile with content.

Ah right, I misread > 1 as >= 1.
Comment 5 Simon Hausmann 2012-01-11 06:38:58 PST
Comment on attachment 121759 [details]
Patch

r=me with the options parameter fix.
Comment 6 Noam Rosenthal 2012-01-11 10:30:08 PST
Created attachment 122048 [details]
Patch
Comment 7 WebKit Review Bot 2012-01-11 11:49:57 PST
Comment on attachment 122048 [details]
Patch

Clearing flags on attachment: 122048

Committed r104728: <http://trac.webkit.org/changeset/104728>
Comment 8 WebKit Review Bot 2012-01-11 11:50:01 PST
All reviewed patches have been landed.  Closing bug.