Bug 84066 - [chromium] Move paintRenderedResultsToCanvas code into DrawingBuffer
: [chromium] Move paintRenderedResultsToCanvas code into DrawingBuffer
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: James Robinson
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-16 13:07 PDT by James Robinson
Modified: 2012-04-16 22:37 PDT (History)
6 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2012-04-16 13:07:36 PDT
[chromium] Move paintRenderedResultsToCanvas code into DrawingBuffer
Comment 1 James Robinson 2012-04-16 13:11:15 PDT
Created attachment 137387 [details]
Patch
Comment 2 James Robinson 2012-04-16 13:20:10 PDT
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 Adrienne Walker 2012-04-16 18:49:33 PDT
Comment on attachment 137387 [details]
Patch

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 James Robinson 2012-04-16 19:13:06 PDT
(In reply to comment #3)
> (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?

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 James Robinson 2012-04-16 19:41:58 PDT
Created attachment 137463 [details]
tweak checks in DrawingBufferChromium
Comment 6 James Robinson 2012-04-16 19:45:57 PDT
Created attachment 137465 [details]
with more of teh compile
Comment 7 Adrienne Walker 2012-04-16 21:46:23 PDT
Comment on attachment 137465 [details]
with more of teh compile

R=me.
Comment 8 WebKit Review Bot 2012-04-16 22:37:04 PDT
Comment on attachment 137465 [details]
with more of teh compile

Clearing flags on attachment: 137465

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