WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
72191
[Chromium] Add support for painting directly to shared memory.
https://bugs.webkit.org/show_bug.cgi?id=72191
Summary
[Chromium] Add support for painting directly to shared memory.
David Reveman
Reported
2011-11-11 15:16:18 PST
Map shared memory buffers prior to painting tile-sized chunks using SkPicture. Allows us to paint directly to shared memory instead of temporary buffers.
Attachments
Patch
(6.74 KB, patch)
2011-11-11 15:31 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Patch
(6.75 KB, patch)
2011-11-15 12:41 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Patch
(5.83 KB, patch)
2011-11-18 15:53 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Patch
(5.81 KB, patch)
2011-11-18 17:04 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Patch
(5.84 KB, patch)
2011-11-20 16:37 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Patch
(5.84 KB, patch)
2011-12-02 10:55 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Patch
(5.84 KB, patch)
2011-12-11 16:33 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (WebKit)
(15.87 KB, patch)
2012-02-16 11:44 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (Chromium)
(2.64 KB, patch)
2012-02-16 11:45 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (WebKit)
(25.00 KB, patch)
2012-02-22 14:34 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (Chromium)
(3.97 KB, patch)
2012-02-22 14:35 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (WebKit)
(26.47 KB, patch)
2012-02-27 10:08 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (Chromium)
(3.97 KB, patch)
2012-02-27 10:09 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (WebKit)
(26.52 KB, patch)
2012-02-29 12:58 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (Chromium)
(4.00 KB, patch)
2012-02-29 12:58 PST
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (WebKit)
(25.39 KB, patch)
2012-03-20 14:57 PDT
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (Chromium)
(3.93 KB, patch)
2012-03-20 14:58 PDT
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (WebKit)
(4.81 KB, patch)
2012-04-02 14:49 PDT
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (Chromium)
(3.96 KB, patch)
2012-04-02 14:50 PDT
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (WebKit)
(4.81 KB, patch)
2012-04-03 09:46 PDT
,
David Reveman
no flags
Details
Formatted Diff
Diff
Prototype (Chromium)
(3.96 KB, patch)
2012-04-03 09:47 PDT
,
David Reveman
no flags
Details
Formatted Diff
Diff
Patch
(38.35 KB, patch)
2012-05-24 12:00 PDT
,
David Reveman
no flags
Details
Formatted Diff
Diff
Show Obsolete
(21)
View All
Add attachment
proposed patch, testcase, etc.
David Reveman
Comment 1
2011-11-11 15:26:17 PST
***
Bug 71023
has been marked as a duplicate of this bug. ***
David Reveman
Comment 2
2011-11-11 15:31:19 PST
Created
attachment 114788
[details]
Patch
James Robinson
Comment 3
2011-11-11 15:42:45 PST
Do we know if this is actually a win?
David Reveman
Comment 4
2011-11-12 17:30:54 PST
(In reply to
comment #3
)
> Do we know if this is actually a win?
This is more important for the ability to implement an out-of-process compositor than performance but eliminating an unnecessary memcpy shouldn't hurt. Shared memory resources are not increased by this change so it seems unlikely it would have a negative impact. Only difference is the shared memory is mapped in the renderer for a bit longer than before. I'd be surprised if that has any performance impact. We could make painting directly to shared memory optional but I don't see a good reason for that.
David Reveman
Comment 5
2011-11-15 12:41:03 PST
Created
attachment 115222
[details]
Patch
Adrienne Walker
Comment 6
2011-11-16 15:37:50 PST
Comment on
attachment 115222
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=115222&action=review
This looks good to me.
> Source/WebCore/platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.cpp:79 > + m_mappedBuffer = NULL;
nit: s/NULL/0/ in WebKit
David Reveman
Comment 7
2011-11-18 15:53:16 PST
Created
attachment 115896
[details]
Patch
David Reveman
Comment 8
2011-11-18 17:04:20 PST
Created
attachment 115911
[details]
Patch
David Reveman
Comment 9
2011-11-20 16:37:27 PST
Created
attachment 116008
[details]
Patch
David Reveman
Comment 10
2011-12-02 10:55:38 PST
Created
attachment 117654
[details]
Patch
David Reveman
Comment 11
2011-12-11 16:33:37 PST
Created
attachment 118717
[details]
Patch
David Reveman
Comment 12
2012-02-16 11:44:47 PST
Created
attachment 127419
[details]
Prototype (WebKit) This is a prototype for how to implement efficient painting directly to shared memory. This is just a proof of concept and is currently missing a framework for determining when shared memory can be reused without affecting pending updates.
David Reveman
Comment 13
2012-02-16 11:45:20 PST
Created
attachment 127420
[details]
Prototype (Chromium)
Dana Jansens
Comment 14
2012-02-16 12:15:43 PST
Comment on
attachment 127419
[details]
Prototype (WebKit) View in context:
https://bugs.webkit.org/attachment.cgi?id=127419&action=review
it's not as scary as i imagined :)
> Source/WebCore/platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.cpp:53 > + if (texture()->mapTransferBuffer())
think this should have width, height arguments? it could just assert on them being wrong even. it's scary to alloc and setPixels with no sizes in sight :) i like how simple this is though!
> Source/WebCore/platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.cpp:70 > + helper->TexSubImage2D(GraphicsContext3D::TEXTURE_2D, 0, destRect.x(), destRect.y(), destRect.width(), destRect.height(), texture()->format(), GraphicsContext3D::UNSIGNED_BYTE, texture()->shmId(), 0, FALSE);
this thing feels like a nice place for an abstraction.. textureUpdater()->updateSharedMemTextureRect() ? or maybe pass texture() to the textureUpdater()->updateTextureRect() and it can do this if{}else{} ? not sure other ideas..
David Reveman
Comment 15
2012-02-16 12:25:03 PST
(In reply to
comment #14
)
> (From update of
attachment 127419
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=127419&action=review
> > it's not as scary as i imagined :) > > > Source/WebCore/platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.cpp:53 > > + if (texture()->mapTransferBuffer()) > > think this should have width, height arguments? it could just assert on them being wrong even. it's scary to alloc and setPixels with no sizes in sight :)
I guess there could be an ASSERT there to check that sourceRect is less than texture dimensions.
> > i like how simple this is though! > > > Source/WebCore/platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.cpp:70 > > + helper->TexSubImage2D(GraphicsContext3D::TEXTURE_2D, 0, destRect.x(), destRect.y(), destRect.width(), destRect.height(), texture()->format(), GraphicsContext3D::UNSIGNED_BYTE, texture()->shmId(), 0, FALSE); > > this thing feels like a nice place for an abstraction.. textureUpdater()->updateSharedMemTextureRect() ? > > or maybe pass texture() to the textureUpdater()->updateTextureRect() and it can do this if{}else{} ?
This is all very temporary. Things will probably look a bit different in a real patch with a proper GraphicsContext3D API.
David Reveman
Comment 16
2012-02-22 14:34:19 PST
Created
attachment 128291
[details]
Prototype (WebKit) Add decent memory management and GPU process coordination.
David Reveman
Comment 17
2012-02-22 14:35:21 PST
Created
attachment 128292
[details]
Prototype (Chromium)
David Reveman
Comment 18
2012-02-27 10:08:15 PST
Created
attachment 129061
[details]
Prototype (WebKit) Rebase and threaded compositor support.
David Reveman
Comment 19
2012-02-27 10:09:59 PST
Created
attachment 129062
[details]
Prototype (Chromium) Rebase.
David Reveman
Comment 20
2012-02-29 12:58:10 PST
Created
attachment 129496
[details]
Prototype (WebKit)
David Reveman
Comment 21
2012-02-29 12:58:43 PST
Created
attachment 129497
[details]
Prototype (Chromium)
David Reveman
Comment 22
2012-03-20 14:57:39 PDT
Created
attachment 132906
[details]
Prototype (WebKit)
David Reveman
Comment 23
2012-03-20 14:58:23 PDT
Created
attachment 132907
[details]
Prototype (Chromium)
David Reveman
Comment 24
2012-04-02 14:49:43 PDT
Created
attachment 135196
[details]
Prototype (WebKit)
David Reveman
Comment 25
2012-04-02 14:50:33 PDT
Created
attachment 135198
[details]
Prototype (Chromium)
Nat Duca
Comment 26
2012-04-02 15:32:15 PDT
Hey David, I'm feeling a bit confused about this bug. I see 20 patches here all labeled prototype. Is there uncertainty about the approach or something? Net/net, how can I provide assistance in order to crisp up the plan and move from prototyping to review and landing?
David Reveman
Comment 27
2012-04-03 00:33:11 PDT
(In reply to
comment #26
)
> Hey David, I'm feeling a bit confused about this bug. I see 20 patches here all labeled prototype.
Sorry about the confusion. I've just been rebasing these patches as there's been some interest in trying them out. I haven't had time to do any real work on this lately but I'm hoping to make some progress here soon.
David Reveman
Comment 28
2012-04-03 09:46:36 PDT
Created
attachment 135345
[details]
Prototype (WebKit) just another rebase..
David Reveman
Comment 29
2012-04-03 09:47:15 PDT
Created
attachment 135346
[details]
Prototype (Chromium) rebase
David Reveman
Comment 30
2012-05-24 12:00:04 PDT
Created
attachment 143867
[details]
Patch Requires this chromium CL:
http://codereview.chromium.org/10440019/
Stephen Chenney
Comment 31
2013-04-12 07:21:57 PDT
https://code.google.com/p/chromium/issues/detail?id=230811
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug