Bug 49560 - [chromium] Avoid copying of SkBitmap in LayerRendererChromium
Summary: [chromium] Avoid copying of SkBitmap in LayerRendererChromium
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-15 13:12 PST by Kenneth Russell
Modified: 2010-11-15 17:02 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.41 KB, patch)
2010-11-15 13:14 PST, Kenneth Russell
jamesr: review+
kbr: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2010-11-15 13:12:26 PST
Grace Kloba found that LayerRendererChromium was inadvertently causing a copy of the SkBitmap data structure for the root layer's canvas. If the bitmap is using an SkPixelRef internally, this can cause getPixels() to return NULL. The fix is to use a const SkBitmap& like ContentLayerChromium already does.
Comment 1 Kenneth Russell 2010-11-15 13:14:30 PST
Created attachment 73925 [details]
Patch
Comment 2 James Robinson 2010-11-15 13:15:36 PST
Comment on attachment 73925 [details]
Patch

Good catch!
Comment 3 Kenneth Russell 2010-11-15 13:17:13 PST
Committed r72026: <http://trac.webkit.org/changeset/72026>
Comment 4 Nico Weber 2010-11-15 16:48:29 PST
Accessing pixels() without calling lockPixels() isn't guaranteed to work.
Comment 5 James Robinson 2010-11-15 16:57:23 PST
Yeah, this really should call accessBitmap(true) as well, no?
Comment 6 Kenneth Russell 2010-11-15 17:02:08 PST
(In reply to comment #4)
> Accessing pixels() without calling lockPixels() isn't guaranteed to work.

This is definitely true for an ordinary SkBitmap, but what about the bitmaps that are managed by an SkDevice?
Comment 7 Kenneth Russell 2010-11-15 17:02:34 PST
(In reply to comment #5)
> Yeah, this really should call accessBitmap(true) as well, no?

No; the bitmap's pixels are only accessed to upload them to OpenGL, and aren't modified.