Normally, layers are manipulated on the WebKit thread, and the changes are synced to the compositing thread during the next accelerated compositing commit operation. However, for overlay layers the ability to manipulate layers on the compositing thread is necessary for adequate user interface responsiveness. Two mechanisms are added: 1. For a layer with a WebKit-thread counterpart, you can temporarily override attributes or add animations whose output override attributes. 2. For a layer with no WebKit-thread counterpart, you can now directly set the compositing thread values for attributes. If you attempt to do this for layers that do have a WebKit-thread counterpart, the compositing thread values will be overwritten at the time of the next commit, which makes the override mechanism more useful there. PR #156812 Please note that this has not been internally reviewed yet, so it needs actual review. And also, it has a dependency that needs to be landed first.
Created attachment 144244 [details] Patch
Created attachment 144247 [details] Patch
Comment on attachment 144244 [details] Patch Hmm, sorry about that, webkit-upload got the bug wrong
Comment on attachment 144244 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144244&action=review > Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.h:77 > + FloatPoint position; > + FloatPoint anchorPoint; > + IntSize bounds; > + TransformationMatrix transform; > + float opacity; > + Vector<RefPtr<LayerAnimation> > m_animations; All members don't have "m_" prefix but m_animations. Should be in same style? More thought: how to avoid people from setting those members directly and forgetting to set the flags? I would rather make them private, and provide read-only get() methods for them.
Comment on attachment 144244 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144244&action=review Ok, can be cleaned up a bit more I think. > Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:394 > + layer->setSuperlayer(this); Would it make sense for setSuperlayer to call removeFromSuperlayer internally? > Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:498 > + if (m_override) { Should probably use early return style here, code will be more readable. >> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.h:77 >> + Vector<RefPtr<LayerAnimation> > m_animations; > > All members don't have "m_" prefix but m_animations. Should be in same style? > > More thought: how to avoid people from setting those members directly and forgetting to set the flags? > > I would rather make them private, and provide read-only get() methods for them. Agree with Yong, m_ is a good prefix to use.
(In reply to comment #5) > (From update of attachment 144244 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144244&action=review > > Ok, can be cleaned up a bit more I think. > > > Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:394 > > + layer->setSuperlayer(this); > > Would it make sense for setSuperlayer to call removeFromSuperlayer internally? That's a great suggestion! However, normally the layer hierarchy on the compositing thread is updated in void LayerWebKitThread::updateLayerHierarchy(), by forcing the compositing thread hierarchy to be equal to the webkit thread hieararchy. That's why there's no sanity checks in the set methods (setsuperlayer, setsublayers). Cleaning that up should be a separate patch I think. > > > Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:498 > > + if (m_override) { > > Should probably use early return style here, code will be more readable. > > >> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.h:77 > >> + Vector<RefPtr<LayerAnimation> > m_animations; > > > > All members don't have "m_" prefix but m_animations. Should be in same style? > > > > More thought: how to avoid people from setting those members directly and forgetting to set the flags? > > > > I would rather make them private, and provide read-only get() methods for them. > > Agree with Yong, m_ is a good prefix to use. New patch forthcoming
Created attachment 144397 [details] Patch
Comment on attachment 144397 [details] Patch Looks good.
Comment on attachment 144397 [details] Patch Rejecting attachment 144397 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 Last 500 characters of output: return self.open(self.click(*args, **kwds)) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 203, in open return self._mech_open(url, data, timeout=timeout) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 255, in _mech_open raise response webkitpy.thirdparty.autoinstalled.mechanize._response.httperror_seek_wrapper: HTTP Error 500: Internal Server Error Full output: http://queues.webkit.org/results/12846288
Comment on attachment 144397 [details] Patch Clearing flags on attachment: 144397 Committed r118709: <http://trac.webkit.org/changeset/118709>
All reviewed patches have been landed. Closing bug.