Bug 75910 – [Qt][Texmap] LayoutTests/compositing/masks/masked-ancestor does not render correctly.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Noam Rosenthal]

Masks do not render correctly with intermediate surfaces.

[Noam Rosenthal]

Created attachment 121759 [details]
Patch

[Simon Hausmann]

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?

[Noam Rosenthal]

> 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.

[Simon Hausmann]

(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.

[Simon Hausmann]

Comment on attachment 121759 [details]
Patch

r=me with the options parameter fix.

[Noam Rosenthal]

Created attachment 122048 [details]
Patch

[WebKit Review Bot]

Comment on attachment 122048 [details]
Patch

Clearing flags on attachment: 122048

Committed r104728: <http://trac.webkit.org/changeset/104728>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.