Summary: | [chromium] Updating WebGraphicsContext3D MemoryAllocation callback to accept a struct with have backbuffer suggestion. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michal Mocny <mmocny> | ||||||||||
Component: | New Bugs | Assignee: | Michal Mocny <mmocny> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, cc-bugs, fishd, jamesr, nduca, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Michal Mocny
2012-03-15 15:47:01 PDT
Created attachment 132137 [details]
Patch
Comment on attachment 132137 [details]
Patch
This is just providing a hint to the compositor about when to drop the backbuffer. The drop extension, and the compositor memory allocation callback coming soon.
Comment on attachment 132137 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132137&action=review > Source/Platform/chromium/public/WebGraphicsContext3D.h:31 > + spurious newline here > Source/Platform/chromium/public/WebGraphicsContext3D.h:135 > + virtual void onMemoryAllocationChanged(const WebGraphicsMemoryAllocation&); passing by value is probably just as (or more) efficient. can you leave a FIXME here indicating that the other version is going away and that this will become pure virtual once the chromium side lands? > Source/Platform/chromium/public/WebGraphicsMemoryAllocation.h:34 > + unsigned int gpuResourceSizeInBytes; here and elsewhere: we spell this type "unsigned", not "unsigned int" > Source/Platform/chromium/public/WebGraphicsMemoryAllocation.h:50 > +inline bool operator==(const WebGraphicsMemoryAllocation& a, const WebGraphicsMemoryAllocation& b) are these necessary? Comment on attachment 132137 [details]
Patch
Other than nits this API is fine with me if it's OK with Nat.
Created attachment 132140 [details]
Patch
Fixed nits. Comment on attachment 132140 [details]
Patch
Looks good. I think Nat's unavailable for a few days, but we can refine when he gets back if there's more feedback.
Comment on attachment 132140 [details]
Patch
Isn't there an ExtensionsChromium change as well?
Yes I did miss adding to ExtensionsChromium, so I will add it as part of the change to have compositor register a callback. Comment on attachment 132140 [details] Patch Rejecting attachment 132140 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: Tools/Scripts/build-webkit', '--debug', '--chromium', '--update-chromium']" exit_code: 2 locationChangedCallbackAdapter:GraphicsContext3DChromium.cpp(.data.rel.ro._ZTVN7WebCore55GraphicsContext3DMemoryAllocationChangedCallbackAdapterE+0x18): error: undefined reference to 'WebKit::WebGraphicsContext3D::WebGraphicsMemoryAllocationChangedCallbackCHROMIUM::onMemoryAllocationChanged(WebKit::WebGraphicsMemoryAllocation)' collect2: ld returned 1 exit status make: *** [out/Debug/DumpRenderTree] Error 1 Full output: http://queues.webkit.org/results/11962166 Created attachment 132287 [details]
Patch
Fixed the build issue and added the Extension3DChromium change. Please wait for approval from fishd@chromium.org, abarth@webkit.org or jamesr@chromium.org before submitting because this patch contains changes to the Chromium platform API. Created attachment 132333 [details]
Patch
Comment on attachment 132333 [details]
Patch
Added a default implementation for allocation callback taking the struct.
Comment on attachment 132333 [details] Patch Clearing flags on attachment: 132333 Committed r111216: <http://trac.webkit.org/changeset/111216> All reviewed patches have been landed. Closing bug. |