Bug 99890 - Fix a hang when combining tile cache layers with preserve-3d or reflections
Summary: Fix a hang when combining tile cache layers with preserve-3d or reflections
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-10-19 16:48 PDT by Simon Fraser (smfr)
Modified: 2012-10-19 17:37 PDT (History)
4 users (show)

See Also:


Attachments
Patch (15.12 KB, patch)
2012-10-19 16:55 PDT, Simon Fraser (smfr)
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2012-10-19 16:48:35 PDT
Fix a hang when combining tile cache layers with preserve-3d or reflections
Comment 1 Simon Fraser (smfr) 2012-10-19 16:49:12 PDT
<rdar://problem/12539560>
Comment 2 Simon Fraser (smfr) 2012-10-19 16:55:53 PDT
Created attachment 169728 [details]
Patch
Comment 3 Dean Jackson 2012-10-19 17:06:57 PDT
Comment on attachment 169728 [details]
Patch

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

I might like to see the changelog describe why preserve-3d and reflection layers hit this.

> Source/WebCore/ChangeLog:15
> +        on the fact that this flag-dirtying functions get called before the later functions

typo this->these

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1445
> -            // Update the properties of m_layer now that we no longer have a structural layer.
> -            updateGeometry(pageScaleFactor, positionRelativeToBase);
> -            updateTransform();
> -            updateChildrenTransform();
> -            
> +            m_uncommittedChanges |= NameChanged
> +                | GeometryChanged
> +                | TransformChanged
> +                | ChildrenTransformChanged
> +                | ChildrenChanged
> +                | BackfaceVisibilityChanged

BackfaceVisibility wasn't here before the change. Is this right? Probably a mistake that it wasn't :)
Comment 4 Simon Fraser (smfr) 2012-10-19 17:15:37 PDT
http://trac.webkit.org/changeset/131964
Comment 5 Darin Adler 2012-10-19 17:27:41 PDT
Comment on attachment 169728 [details]
Patch

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

> Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:333
> +    ASSERT(m_layer.get() != layer->m_layer.get());

This kind of thing is supposed to work without the get().

> Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:341
> +    ASSERT(m_layer.get() != layer->m_layer.get());

Ditto.

> Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:349
> +    ASSERT(m_layer.get() != layer->m_layer.get());

Again.
Comment 6 Simon Fraser (smfr) 2012-10-19 17:37:12 PDT
(In reply to comment #5)
> (From update of attachment 169728 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169728&action=review
> 
> > Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:333
> > +    ASSERT(m_layer.get() != layer->m_layer.get());
> 
> This kind of thing is supposed to work without the get().
> 
> > Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:341
> > +    ASSERT(m_layer.get() != layer->m_layer.get());
> 
> Ditto.
> 
> > Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:349
> > +    ASSERT(m_layer.get() != layer->m_layer.get());
> 
> Again.

Fixed in http://trac.webkit.org/changeset/131966.