[Webkit-unassigned] [Bug 54921] [Chromium] Issue 60965: Layout test fast/canvas/setWidthResetAfterForcedRender.html is failing on GPU path

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54921





--- Comment #13 from Naoki Takano <takano.naoki at gmail.com>  2011-02-25 14:46:01 PST ---
James,

Could you give me your comment?

(In reply to comment #12)
> James,
> 
> Could you give me your comment?
> 
> Thanks,
> 
> (In reply to comment #11)
> > James,
> > 
> > I confirmed Stephen's fix and it looks Ok with Chrome.
> > 
> > But it doesn't with DumpRenderTree.
> > 
> > As you know, we can run the test like following,
> > ./webkit/tools/layout_tests/run_webkit_tests.sh  --accelerated-2d-canvas fast/canvas/setWidthResetAfterForcedRender.html
> > 
> > It fails as following,
> >          RenderText {#text} at (0,0) size 176x19
> >            text run at (0,0) width 176: "Actual output: (blank canvas)"
> >        RenderBlock (anonymous) at (0,152) size 784x55
> > -        RenderHTMLCanvas {CANVAS} at (0,0) size 100x50
> >          RenderText {#text} at (0,0) size 0x0
> >          RenderText {#text} at (0,0) size 0x0
> > +layer at (8,168) size 100x50
> > +  RenderHTMLCanvas {CANVAS} at (0,0) size 100x50
> > 
> > As you see, rendering tree is different.
> > 
> > That's why I said
> > > > For example, in setWidthResetAfterForce, the canvas element becomes the same layer of the root. That causes the test failure. That is why I change this part.
> > 
> > If we have to fix completely, we have to consider DumpRenderTree part, right?
> > 
> > What do you think?
> > 
> > (In reply to comment #10)
> > > James,
> > > 
> > > Thank you for your comment. I'll make sure Stephan's patch. Still I'm a beginner for WebKit, I have to study a lot... So I'm sorry if my patch is far from the correct fix;-)
> > > 
> > > But thank you for your dedication every time.
> > > 
> > > (In reply to comment #9)
> > > > (In reply to comment #7)
> > > > > James,
> > > > > 
> > > > > Thank you for your review.
> > > > > 
> > > > > As you know, requiresLayer() function is called bye
> > > > > RenderBoxModelObject::styleDidChange()
> > > > > RenderObject::updateFillImages()
> > > > > 
> > > > > If the returned values are true, the rendering tree becomes different from the non-accelerated rendering trees. The canvas layer order is different from non-accelerated.
> > > > 
> > > > Yes - this is expected.  An accelerated canvas requires a layer whereas a non-accelerated canvas may or may not need its own layer.  Different does not mean wrong.
> > > > 
> > > > > 
> > > > > For example, in setWidthResetAfterForce, the canvas element becomes the same layer of the root. That causes the test failure. That is why I change this part.
> > > > 
> > > > I'm not sure what you mean by 'becomes the same layer of the root' - could you expand?
> > > > 
> > > > > 
> > > > > I want to make sure the layer purpose. I thought this looked like the very special layer to overlay the rendered images in partial. Is this correct? Or is there another purpose for the layer?
> > > > 
> > > > The RenderLayer is required because accelerated canvas require their own composited layer.  See RenderLayerCompositor::requiresCompositingForCanvas() to see a similar check.
> > > > 
> > > > I'm pretty sure Stephen fixed this test with https://bugs.webkit.org/show_bug.cgi?id=54561 at http://trac.webkit.org/changeset/78922 - could you confirm if the test still fails after that revision?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.