The origins of the command buffer service implementation are arcane and overly complex. Many classes are involved, with curious dependencies between them, and duplication of responsibility.
Some examples:
- CommandParser, CommmandExecutor and CommandBufferService are always 1:1 with each other, but duplicate a lot of information (e.g. the "get" pointer is in at least 4 different places, and updated via callbacks and virtual functions)
- layering between those as well as CommonDecoder/GLES2Decoder and GpuCommandBufferStub/InProcessCommandBuffer is unclear/broken, with (over-)use of callbacks and ill-defined interfaces, and results in inconsistencies between out-of-process vs in-process implementations
- CommandExecutor is strongly coupled to GLES2Decoder for no good reason
- setting up a command buffer service for e.g. tests is complicated, and essentially cargo-culted all over the place, making maintenance harder
- CommandBufferService does both too much and too little. It wants to implement CommandBuffer (the client interface for GLES2Implementation), but at the same time provide a common implementation of the service-side. However, just by itself, calling CommandBufferService::Flush does nothing at all, since it needs a CommmandExecutor/CommandParser to be hooked up to it via callbacks.
All this makes it even harder to build different command-buffer backends (e.g. dedicated skia).
My proposal (so far):
- eliminate CommandParser, merging into CommandExecutor
- eliminate CommandBufferEngine (replace with CommandBufferServiceBase as the client interface in CommonDecoder)
- make CommandBufferService focus on the service-side, and move all the client-side logic (used for tests) into a CommandBufferDirect, that would set everything up together into a usable CommandBuffer. Tests migrate to use CommandBufferDirect. Merge CommandBufferService and CommandExecutor to remove unnecessary callback-based logic
- provide a client interface for GLES2Decoder instead of the multitude of callbacks
I started on all this, and so far this leads to significant code reduction/de-duplication.
Comment 1 by bugdroid1@chromium.org
, May 18 2017