Bug 84066 - [chromium] Move paintRenderedResultsToCanvas code into DrawingBuffer
: [chromium] Move paintRenderedResultsToCanvas code into DrawingBuffer
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2012-04-16 13:07 PST by
Modified: 2012-04-16 22:37 PST (History)


Attachments
Patch (5.93 KB, patch)
2012-04-16 13:11 PST, James Robinson
no flags Review Patch | Details | Formatted Diff | Diff
tweak checks in DrawingBufferChromium (6.00 KB, patch)
2012-04-16 19:41 PST, James Robinson
no flags Review Patch | Details | Formatted Diff | Diff
with more of teh compile (6.00 KB, patch)
2012-04-16 19:45 PST, James Robinson
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-04-16 13:07:36 PST
[chromium] Move paintRenderedResultsToCanvas code into DrawingBuffer
------- Comment #1 From 2012-04-16 13:11:15 PST -------
Created an attachment (id=137387) [details]
Patch
------- Comment #2 From 2012-04-16 13:20:10 PST -------
Tested on the layout tests and by manually checking printing of san angeles and http://trac.webkit.org/export/114291/trunk/LayoutTests/fast/canvas/webgl/webgl-composite-modes.html.

On http://trac.webkit.org/export/114291/trunk/LayoutTests/fast/canvas/webgl/webgl-composite-modes.html, after printing the 4 leftmost canvases (preservesDrawingBuffer==false) vanish.  This happens with or without this patch.
------- Comment #3 From 2012-04-16 18:49:33 PST -------
(From update of attachment 137387 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=137387&action=review

> Source/WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:173
> +    if (!m_platformLayer || !m_platformLayer->drawsContent())
> +        return;

This code made more sense in WebGLLayerChromium.  Would it make more sense just to check for context lost explicitly than go back to the platform layer, since that really doesn't have anything to do with this function anymore?
------- Comment #4 From 2012-04-16 19:13:06 PST -------
(In reply to comment #3)
> (From update of attachment 137387 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=137387&action=review
> 
> > Source/WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:173
> > +    if (!m_platformLayer || !m_platformLayer->drawsContent())
> > +        return;
> 
> This code made more sense in WebGLLayerChromium.  Would it make more sense just to check for context lost explicitly than go back to the platform layer, since that really doesn't have anything to do with this function anymore?

This check in particular?  This check is actually kind of silly - we never get here without having a layer (see WebGLRenderingContext::paintRenderingResultsToCanvas()) and I agree it'd be better to check explicitly for a lost context here.
------- Comment #5 From 2012-04-16 19:41:58 PST -------
Created an attachment (id=137463) [details]
tweak checks in DrawingBufferChromium
------- Comment #6 From 2012-04-16 19:45:57 PST -------
Created an attachment (id=137465) [details]
with more of teh compile
------- Comment #7 From 2012-04-16 21:46:23 PST -------
(From update of attachment 137465 [details])
R=me.
------- Comment #8 From 2012-04-16 22:37:04 PST -------
(From update of attachment 137465 [details])
Clearing flags on attachment: 137465

Committed r114344: <http://trac.webkit.org/changeset/114344>
------- Comment #9 From 2012-04-16 22:37:11 PST -------
All reviewed patches have been landed.  Closing bug.