|
|
Chromium Code Reviews
DescriptionExpose --disable-gpu-compositing browser arg to Blink
Currently in Blink, there is no indication whether a compositor is GPU
accelerated or software. This CL plumbs it to Blink, so that we can
query whether --disable-gpu-compositing or --disable-gpu is set or not.
BUG=650795
Committed: https://crrev.com/c234ff9ce6b4ce3c1561dc99ef30837d4000101e
Cr-Commit-Position: refs/heads/master@{#422672}
Patch Set 1 #Patch Set 2 : add to RuntimeEnabledFeatures.in #Patch Set 3 : set default gpu compositing to be true #Patch Set 4 : rebse after Blink Format #
Total comments: 1
Messages
Total messages: 39 (12 generated)
xidachen@chromium.org changed reviewers: + esprehn@chromium.org, junov@chromium.org
PTAL
Not lgtm This is not how gpu features work. You cannot tell whether any gpu feature is active just by looking at command line arguments. The graphics hardware and driver must pass a number of feature detection and blacklisting tests, then the compositor may have additional criteria of its own. What you need to do is query the state of the compositor. I suggest you ask enne@ or danakj@ for guidance on how you might implement this query from blink.
Description was changed from ========== Expose --disable-gpu-compositing browser arg to Blink Currently in Blink, there is no indication whether a compositor is GPU accelerated or software. This CL plumbs it to Blink, so that we can query whether --disable-gpu-compositing or --disable-gpu is set or not. BUG=650795 ========== to ========== Expose --disable-gpu-compositing browser arg to Blink Currently in Blink, there is no indication whether a compositor is GPU accelerated or software. This CL plumbs it to Blink, so that we can query whether --disable-gpu-compositing or --disable-gpu is set or not. BUG=650795 ==========
xidachen@chromium.org changed reviewers: + danakj@chromium.org, jbauman@chromium.org
xidachen@chromium.org changed reviewers: + enne@chromium.org
+danakj@, jbauman@, and enne@ My original idea for CL is to find a reliable way to tell whether compositor is GPU or software. The WebGraphicsContext3DProvider::isSoftwareRendering() is not indicating that, because it will return 0 even if I pass --disable-gpu-compositing arg. Based on offline discussion with danakj@ and jbauman@, we thought that the most direct way is to detect whether there exists flag --disable-gpu-compositing or --disable-gpu or not. But maybe that's not accurate enough.
On 2016/09/28 15:09:19, xidachen wrote: > +danakj@, jbauman@, and enne@ > > My original idea for CL is to find a reliable way to tell whether compositor is > GPU or software. The WebGraphicsContext3DProvider::isSoftwareRendering() is not > indicating that, because it will return 0 even if I pass > --disable-gpu-compositing arg. Based on offline discussion with danakj@ and > jbauman@, we thought that the most direct way is to detect whether there exists > flag --disable-gpu-compositing or --disable-gpu or not. But maybe that's not > accurate enough. I guess the question is: When the browser constructs the list of flags to be passed to the renderer, does that happen downstream of *all* logic that might enable or disable gpu compositing? I think not, but I could be wrong.
On 2016/09/28 15:16:15, Justin Novosad wrote: > On 2016/09/28 15:09:19, xidachen wrote: > > +danakj@, jbauman@, and enne@ > > > > My original idea for CL is to find a reliable way to tell whether compositor > is > > GPU or software. The WebGraphicsContext3DProvider::isSoftwareRendering() is > not > > indicating that, because it will return 0 even if I pass > > --disable-gpu-compositing arg. Based on offline discussion with danakj@ and > > jbauman@, we thought that the most direct way is to detect whether there > exists > > flag --disable-gpu-compositing or --disable-gpu or not. But maybe that's not > > accurate enough. > > I guess the question is: When the browser constructs the list of flags to be > passed to the renderer, does that happen downstream of *all* logic that might > enable or disable gpu compositing? > I think not, but I could be wrong. Yeah, --disable-gpu-compositing is the end result of the browser's decisionmaking about whether the renderer should have GPU compositing. The code in blink will also need to check WebGraphicsContext3DProvider::isSoftwareRendering() after trying to create a GPU channel, but that's an additional backup to this check. See RenderThreadImpl::CreateCompositorFrameSink for how the renderer compositor decides.
Why do you need to know this inside blink? Historically we've avoided exposing this because we didn't want people to branch web platform behavior on it. -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Why do you need to know this inside blink? Historically we've avoided exposing this because we didn't want people to branch web platform behavior on it. -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
On 2016/09/28 23:26:03, esprehn wrote: > Why do you need to know this inside blink? Historically we've avoided > exposing this because we didn't want people to branch web platform behavior > on it. > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org. esprehn@: When implementing OffscreenCanvas's commit API, we are trying to prepare a cc::CompositorFrame and submit the CompositorFrame directly to the browser. Preparing the CompositorFrame happens on Blink. We are trying to implementing several different case scenarios: 1. Both canvas and compositor are gpu accelerated. 2. Both canvas and compositor are software. 3. Canvas is software but compositor is GPU. To implement case 2 and 3 we would need a flag to tell us whether compositor is GPU or not, because the code path will be completely different.
On 2016/09/29 at 00:07:47, xidachen wrote: > On 2016/09/28 23:26:03, esprehn wrote: > > Why do you need to know this inside blink? Historically we've avoided > > exposing this because we didn't want people to branch web platform behavior > > on it. > > > > esprehn@: When implementing OffscreenCanvas's commit API, we are trying to prepare a cc::CompositorFrame and submit the CompositorFrame directly to the browser. Preparing the CompositorFrame happens on Blink. We are trying to implementing several different case scenarios: > 1. Both canvas and compositor are gpu accelerated. > 2. Both canvas and compositor are software. > 3. Canvas is software but compositor is GPU. > > To implement case 2 and 3 we would need a flag to tell us whether compositor is GPU or not, because the code path will be completely different. commit() in an offscreen canvas needs to pass the data back to the main thread btw, since doing drawImage(canvas) where canvas has passed control to an OffscreenCanvas in a worker still needs to draw the contents. This all feels like premature optimization though. We don't even have the CSS part of offscreen canvas working yet, I'm not sure we should be focusing on trying to submit textures directly yet. Also doesn't that mean we could end up with non-atomic commits where the canvas is updating even though the main thread is janked? Is the UI compositor going to just ignore the OffscreenCanvas frames until the main thread commits?
On 2016/09/29 00:43:38, esprehn wrote: > > commit() in an offscreen canvas needs to pass the data back to the main thread > btw, since doing drawImage(canvas) where canvas has passed control to an > OffscreenCanvas in a worker still needs to draw the contents. That is not how commit() works. On the main thread, you have a <canvas> element that lives in the DOM. You call transferControlToOffscreen() on the canvas in order to get an OffscreenCanvas object that is a proxy for the <canvas>. You transfer the OffscreenCanvas to a Worker. Then you can draw to the OffscreenCanvas from the worker, and when you call commit(), the bitmap in the OffscreenCanvas get pushed to a layer in the UI compositor that corresponds to the layout rect of the placeholder <canvas>, without going through the main thread. The commit flow bypasses the regular paint flow completely. The way the implementation achieves this is that when transferControlToOffscreen is called, the <canvas> element sets up a SurfaceLayer so that the OffscreenCanvas can channel frames directly to the UI compositor. > > This all feels like premature optimization though. It is not an optimization, we need to do this just to get commit() to work. If we pump SharedBitmaps to a gpu compositor, the commit won't work. So in the case where the canvas is not accelerated, we need to determine whether or not the committed bitmap needs to be packaged as a shared bitmap or as a texture mailbox so that the surface layer will accept it. > We don't even have the CSS > part of offscreen canvas working yet, I'm not sure we should be focusing on > trying to submit textures directly yet. There is no CSS on OffscreenCanvas, it is not a DOM element. All the styling happens on the placeholder <canvas> element that resides on the main thread. > Also doesn't that mean we could end up > with non-atomic commits where the canvas is updating even though the main thread > is janked? Yes, this is by design. OffscreenCanvas commits are not synchronized with DOM updates. One of the objectives of this feature is to decouple animation updates from the main thread's event loop to avoid jank. > Is the UI compositor going to just ignore the OffscreenCanvas frames > until the main thread commits? Correct. If that race condition occurs, that is exactly what will happen, by design.
based on jbauman's response, I retract my previous objection. lgtm for Source/platform
On 2016/09/29 at 14:10:13, junov wrote: > On 2016/09/29 00:43:38, esprehn wrote: > > > > commit() in an offscreen canvas needs to pass the data back to the main thread > > btw, since doing drawImage(canvas) where canvas has passed control to an > > OffscreenCanvas in a worker still needs to draw the contents. > > That is not how commit() works. On the main thread, you have a <canvas> element that lives in the DOM. You call transferControlToOffscreen() on the canvas in order to get an OffscreenCanvas object that is a proxy for the <canvas>. You transfer the OffscreenCanvas to a Worker. Then you can draw to the OffscreenCanvas from the worker, and when you call commit(), the bitmap in the OffscreenCanvas get pushed to a layer in the UI compositor that corresponds to the layout rect of the placeholder <canvas>, without going through the main thread. The commit flow bypasses the regular paint flow completely. The way the implementation achieves this is that when transferControlToOffscreen is called, the <canvas> element sets up a SurfaceLayer so that the OffscreenCanvas can channel frames directly to the UI compositor. That means the spec has critically regressed from my spec feedback when it was being designed. If you can't draw the canvas to another canvas you break a bunch of important use cases. The texture needs to be available on main (even if asynchronously). Just like scrolling and animations that run async are available on main eventually. > > > > > This all feels like premature optimization though. > > It is not an optimization, we need to do this just to get commit() to work. If we pump SharedBitmaps to a gpu compositor, the commit won't work. So in the case where the canvas is not accelerated, we need to determine whether or not the committed bitmap needs to be packaged as a shared bitmap or as a texture mailbox so that the surface layer will accept it. That seems like there's a missing abstraction somewhere in the system then. The downstream features in blink shouldn't need to care about if we're using software or hardware. That should happen upstream when we submit the display list and layer data. ex. Safari doesn't care at all what mode CA is running in, it handles the software vs hardware difference for them. > > > We don't even have the CSS > > part of offscreen canvas working yet, I'm not sure we should be focusing on > > trying to submit textures directly yet. > > There is no CSS on OffscreenCanvas, it is not a DOM element. All the styling happens on the placeholder <canvas> element that resides on the main thread. Then why does any of the CSSValue code need to be thread safe? All those patches adding TLS for various things are trying to make using CSS in a worker work. > > > Also doesn't that mean we could end up > > with non-atomic commits where the canvas is updating even though the main thread > > is janked? > > Yes, this is by design. OffscreenCanvas commits are not synchronized with DOM updates. One of the objectives of this feature is to decouple animation updates from the main thread's event loop to avoid jank. Okay, where is the raf signal for the worker? > > > Is the UI compositor going to just ignore the OffscreenCanvas frames > > until the main thread commits? > > Correct. If that race condition occurs, that is exactly what will happen, by design.
On Thu, Sep 29, 2016 at 9:22 AM, <esprehn@chromium.org> wrote: > On 2016/09/29 at 14:10:13, junov wrote: > > On 2016/09/29 00:43:38, esprehn wrote: > > > > > > commit() in an offscreen canvas needs to pass the data back to the main > thread > > > btw, since doing drawImage(canvas) where canvas has passed control to > an > > > OffscreenCanvas in a worker still needs to draw the contents. > > > > That is not how commit() works. On the main thread, you have a <canvas> > element that lives in the DOM. You call transferControlToOffscreen() on the > canvas in order to get an OffscreenCanvas object that is a proxy for the > <canvas>. You transfer the OffscreenCanvas to a Worker. Then you can draw > to > the OffscreenCanvas from the worker, and when you call commit(), the > bitmap in > the OffscreenCanvas get pushed to a layer in the UI compositor that > corresponds > to the layout rect of the placeholder <canvas>, without going through the > main > thread. The commit flow bypasses the regular paint flow completely. The way > the implementation achieves this is that when transferControlToOffscreen is > called, the <canvas> element sets up a SurfaceLayer so that the > OffscreenCanvas > can channel frames directly to the UI compositor. > > That means the spec has critically regressed from my spec feedback when it > was > being designed. If you can't draw the canvas to another canvas you break a > bunch > of important use cases. The texture needs to be available on main (even if > asynchronously). Just like scrolling and animations that run async are > available > on main eventually. > > > > > > > > > This all feels like premature optimization though. > > > > It is not an optimization, we need to do this just to get commit() to > work. > If we pump SharedBitmaps to a gpu compositor, the commit won't work. So in > the > case where the canvas is not accelerated, we need to determine whether or > not > the committed bitmap needs to be packaged as a shared bitmap or as a > texture > mailbox so that the surface layer will accept it. > > That seems like there's a missing abstraction somewhere in the system > then. The > downstream features in blink shouldn't need to care about if we're using > software or hardware. That should happen upstream when we submit the > display > list and layer data. ex. Safari doesn't care at all what mode CA is > running in, > it handles the software vs hardware difference for them. > There is no display list or layer data in this case. What we have is a bitmap or a texture. If canvas is running in 2d mode (for eg gpu canvas is blacklisted, it's too big, etc), then it will have a bitmap. The compositor may still be using GPU, so it needs to provide a texture. (This is 2d canvas disabled, gpu compositing enabled platform flags) - In regular canvas we give a display list in this case. But offscreen canvas is not going through a layer compositor to do raster/upload. The alternative would be to spin up a whole other layer compositor instance and submit display lists to that which is a significantly different design. - In the future we're looking at moving raster out of process. It's possible offscreen canvas could use the same APIs to do out-of-process raster. If canvas is running in 3d mode, then it will have a texture. However it may be using swiftshader to emulate GPU due to blacklist, and the compositor is software. It needs to do a readback and provide a bitmap. (This is 2d canvas enabled and gpu compositing disabled platform flags) Or the gpu process may have crashed too many times and we've fallen back to either software mode or to swiftshader. Here the command line didn't say anymore, but the presence of a WebGraphicsContext3DProvider and the isSoftwareRendering check on it tells us the behaviour of the compositor and what should be submitted, as above. Getting video to go directly to the browser instead of being gated on the layer compositor would be much simpler and more efficient, and would depend on knowing this state also to tell if it should be sending a bitmap or texture. > > We don't even have the CSS > > > part of offscreen canvas working yet, I'm not sure we should be > focusing on > > > trying to submit textures directly yet. > > > > There is no CSS on OffscreenCanvas, it is not a DOM element. All the > styling > happens on the placeholder <canvas> element that resides on the main > thread. > > Then why does any of the CSSValue code need to be thread safe? All those > patches > adding TLS for various things are trying to make using CSS in a worker > work. > > > > > > Also doesn't that mean we could end up > > > with non-atomic commits where the canvas is updating even though the > main > thread > > > is janked? > > > > Yes, this is by design. OffscreenCanvas commits are not synchronized > with DOM > updates. One of the objectives of this feature is to decouple animation > updates > from the main thread's event loop to avoid jank. > > Okay, where is the raf signal for the worker? > > > > > > Is the UI compositor going to just ignore the OffscreenCanvas frames > > > until the main thread commits? > > > > Correct. If that race condition occurs, that is exactly what will > happen, by > design. > > https://codereview.chromium.org/2377883002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On Thu, Sep 29, 2016 at 9:22 AM, <esprehn@chromium.org> wrote: > On 2016/09/29 at 14:10:13, junov wrote: > > On 2016/09/29 00:43:38, esprehn wrote: > > > > > > commit() in an offscreen canvas needs to pass the data back to the main > thread > > > btw, since doing drawImage(canvas) where canvas has passed control to > an > > > OffscreenCanvas in a worker still needs to draw the contents. > > > > That is not how commit() works. On the main thread, you have a <canvas> > element that lives in the DOM. You call transferControlToOffscreen() on the > canvas in order to get an OffscreenCanvas object that is a proxy for the > <canvas>. You transfer the OffscreenCanvas to a Worker. Then you can draw > to > the OffscreenCanvas from the worker, and when you call commit(), the > bitmap in > the OffscreenCanvas get pushed to a layer in the UI compositor that > corresponds > to the layout rect of the placeholder <canvas>, without going through the > main > thread. The commit flow bypasses the regular paint flow completely. The way > the implementation achieves this is that when transferControlToOffscreen is > called, the <canvas> element sets up a SurfaceLayer so that the > OffscreenCanvas > can channel frames directly to the UI compositor. > > That means the spec has critically regressed from my spec feedback when it > was > being designed. If you can't draw the canvas to another canvas you break a > bunch > of important use cases. The texture needs to be available on main (even if > asynchronously). Just like scrolling and animations that run async are > available > on main eventually. > > > > > > > > > This all feels like premature optimization though. > > > > It is not an optimization, we need to do this just to get commit() to > work. > If we pump SharedBitmaps to a gpu compositor, the commit won't work. So in > the > case where the canvas is not accelerated, we need to determine whether or > not > the committed bitmap needs to be packaged as a shared bitmap or as a > texture > mailbox so that the surface layer will accept it. > > That seems like there's a missing abstraction somewhere in the system > then. The > downstream features in blink shouldn't need to care about if we're using > software or hardware. That should happen upstream when we submit the > display > list and layer data. ex. Safari doesn't care at all what mode CA is > running in, > it handles the software vs hardware difference for them. > There is no display list or layer data in this case. What we have is a bitmap or a texture. If canvas is running in 2d mode (for eg gpu canvas is blacklisted, it's too big, etc), then it will have a bitmap. The compositor may still be using GPU, so it needs to provide a texture. (This is 2d canvas disabled, gpu compositing enabled platform flags) - In regular canvas we give a display list in this case. But offscreen canvas is not going through a layer compositor to do raster/upload. The alternative would be to spin up a whole other layer compositor instance and submit display lists to that which is a significantly different design. - In the future we're looking at moving raster out of process. It's possible offscreen canvas could use the same APIs to do out-of-process raster. If canvas is running in 3d mode, then it will have a texture. However it may be using swiftshader to emulate GPU due to blacklist, and the compositor is software. It needs to do a readback and provide a bitmap. (This is 2d canvas enabled and gpu compositing disabled platform flags) Or the gpu process may have crashed too many times and we've fallen back to either software mode or to swiftshader. Here the command line didn't say anymore, but the presence of a WebGraphicsContext3DProvider and the isSoftwareRendering check on it tells us the behaviour of the compositor and what should be submitted, as above. Getting video to go directly to the browser instead of being gated on the layer compositor would be much simpler and more efficient, and would depend on knowing this state also to tell if it should be sending a bitmap or texture. > > We don't even have the CSS > > > part of offscreen canvas working yet, I'm not sure we should be > focusing on > > > trying to submit textures directly yet. > > > > There is no CSS on OffscreenCanvas, it is not a DOM element. All the > styling > happens on the placeholder <canvas> element that resides on the main > thread. > > Then why does any of the CSSValue code need to be thread safe? All those > patches > adding TLS for various things are trying to make using CSS in a worker > work. > > > > > > Also doesn't that mean we could end up > > > with non-atomic commits where the canvas is updating even though the > main > thread > > > is janked? > > > > Yes, this is by design. OffscreenCanvas commits are not synchronized > with DOM > updates. One of the objectives of this feature is to decouple animation > updates > from the main thread's event loop to avoid jank. > > Okay, where is the raf signal for the worker? > > > > > > Is the UI compositor going to just ignore the OffscreenCanvas frames > > > until the main thread commits? > > > > Correct. If that race condition occurs, that is exactly what will > happen, by > design. > > https://codereview.chromium.org/2377883002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
On 2016/09/29 16:22:28, esprehn wrote: > > That means the spec has critically regressed from my spec feedback when it was > being designed. If you can't draw the canvas to another canvas you break a bunch > of important use cases. The texture needs to be available on main (even if > asynchronously). Just like scrolling and animations that run async are available > on main eventually. Those use cases are addressed by a different API. commit() is the fast as possible low jank way of using OffscreenCanvas. The other way of using OffscreenCanvas in a Worker is to call OffscreenCanvas.transferToImageBitmap(), which gives you a snapshot if the canvas's bitmap in the form of an ImageBitmap object. This ImageBitmap object can be transferred to the main thread, and on the main thread you can do many things with it, including blitting it to a <canvas> element. > > Then why does any of the CSSValue code need to be thread safe? All those patches > adding TLS for various things are trying to make using CSS in a worker work. Ah, right... It is not for CSS per se, it is because the OffscreenCanvasRenderingContext2D API has a few attributes that need to be evaluated based on CSS parsing rules. In particular, fillStyle and strokeStyle attributes can be assigned literal color values using any of the formats that the CSS parser knows how to parse '#fff', 'white', 'rgb(255, 255, 255)', etc. We wanted to avoid writing a whole new independent parser for this, so we found ways to invoke the parts of the CSS parser that we needed on a worker. Its the same thing for the 'filter' attribute. > > Okay, where is the raf signal for the worker? That is currently our missing link. It will be worked around in early demos by forwarding the main thread's rAF (using postMessage to forward clock ticks), but this is not ideal and it requires some rate limiting trickery in JS to avoid accumulating a multi-frame backlog. Exposing rAF in Workers is on the agenda. We're just not there yet in the implementation, and we still need to figure out how to spec it.
esprehn@: gentle ping, could we please move forward?
esprehn@chromium.org changed reviewers: + vollick@chromium.org
+vollick I think we need to have a meeting. There's some missing abstraction here, blink should never need to think about if we're in a software vs hardware mode. The cc or platform abstractions should hide it from core/ and modules/, and the correct graphics backend should be picked for you. In the same way you don't need to think about how DirectX or CoreGraphics is actually making your pixels. There shouldn't be any code inside core/ or modules/ trying to branch on if we have gpu compositing enabled, and I'm very uncomfortable adding this flag. That said I'll lgtm this so you can continue forward for now, but I think we need to resolve this. I'm very worried about other stuff depending on this flag.
On Mon, Oct 3, 2016 at 5:27 PM, <esprehn@chromium.org> wrote: > +vollick > > I think we need to have a meeting. There's some missing abstraction here, > blink > should never need to think about if we're in a software vs hardware mode. > The cc > or platform abstractions should hide it from core/ and modules/, and the > correct > graphics backend should be picked for you. > platform/ needs to know it, not core/ and modules/. > In the same way you don't need to > think about how DirectX or CoreGraphics is actually making your pixels. > > There shouldn't be any code inside core/ or modules/ trying to branch on > if we > have gpu compositing enabled, and I'm very uncomfortable adding this flag. > > That said I'll lgtm this so you can continue forward for now, but I think > we > need to resolve this. I'm very worried about other stuff depending on this > flag. > > https://codereview.chromium.org/2377883002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On Mon, Oct 3, 2016 at 5:27 PM, <esprehn@chromium.org> wrote: > +vollick > > I think we need to have a meeting. There's some missing abstraction here, > blink > should never need to think about if we're in a software vs hardware mode. > The cc > or platform abstractions should hide it from core/ and modules/, and the > correct > graphics backend should be picked for you. > platform/ needs to know it, not core/ and modules/. > In the same way you don't need to > think about how DirectX or CoreGraphics is actually making your pixels. > > There shouldn't be any code inside core/ or modules/ trying to branch on > if we > have gpu compositing enabled, and I'm very uncomfortable adding this flag. > > That said I'll lgtm this so you can continue forward for now, but I think > we > need to resolve this. I'm very worried about other stuff depending on this > flag. > > https://codereview.chromium.org/2377883002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
The CQ bit was checked by xidachen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/10/04 at 00:30:14, danakj wrote: > On Mon, Oct 3, 2016 at 5:27 PM, <esprehn@chromium.org> wrote: > > > +vollick > > > > I think we need to have a meeting. There's some missing abstraction here, > > blink > > should never need to think about if we're in a software vs hardware mode. > > The cc > > or platform abstractions should hide it from core/ and modules/, and the > > correct > > graphics backend should be picked for you. > > > > platform/ needs to know it, not core/ and modules/. There's no restriction on that with flags though, once you add this it's exposed to the whole engine.
On Mon, Oct 3, 2016 at 5:33 PM, <esprehn@chromium.org> wrote: > On 2016/10/04 at 00:30:14, danakj wrote: > > On Mon, Oct 3, 2016 at 5:27 PM, <esprehn@chromium.org> wrote: > > > > > +vollick > > > > > > I think we need to have a meeting. There's some missing abstraction > here, > > > blink > > > should never need to think about if we're in a software vs hardware > mode. > > > The cc > > > or platform abstractions should hide it from core/ and modules/, and > the > > > correct > > > graphics backend should be picked for you. > > > > > > > platform/ needs to know it, not core/ and modules/. > > There's no restriction on that with flags though, once you add this it's > exposed > to the whole engine. > We could have a Platform function that checks the command line flag or something if that would make you happier. > > > https://codereview.chromium.org/2377883002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
On Mon, Oct 3, 2016 at 5:33 PM, <esprehn@chromium.org> wrote: > On 2016/10/04 at 00:30:14, danakj wrote: > > On Mon, Oct 3, 2016 at 5:27 PM, <esprehn@chromium.org> wrote: > > > > > +vollick > > > > > > I think we need to have a meeting. There's some missing abstraction > here, > > > blink > > > should never need to think about if we're in a software vs hardware > mode. > > > The cc > > > or platform abstractions should hide it from core/ and modules/, and > the > > > correct > > > graphics backend should be picked for you. > > > > > > > platform/ needs to know it, not core/ and modules/. > > There's no restriction on that with flags though, once you add this it's > exposed > to the whole engine. > We could have a Platform function that checks the command line flag or something if that would make you happier. > > > https://codereview.chromium.org/2377883002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/10/04 00:36:21, danakj wrote: > On Mon, Oct 3, 2016 at 5:33 PM, <mailto:esprehn@chromium.org> wrote: > > > On 2016/10/04 at 00:30:14, danakj wrote: > > > On Mon, Oct 3, 2016 at 5:27 PM, <mailto:esprehn@chromium.org> wrote: > > > > > > > +vollick > > > > > > > > I think we need to have a meeting. There's some missing abstraction > > here, > > > > blink > > > > should never need to think about if we're in a software vs hardware > > mode. > > > > The cc > > > > or platform abstractions should hide it from core/ and modules/, and > > the > > > > correct > > > > graphics backend should be picked for you. > > > > > > > > > > platform/ needs to know it, not core/ and modules/. > > > > There's no restriction on that with flags though, once you add this it's > > exposed > > to the whole engine. > > > > We could have a Platform function that checks the command line flag or > something if that would make you happier. I defer to Elliott, but accessing this via Platform makes sense to me. > > > > > > > > https://codereview.chromium.org/2377883002/ > > > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org.
The CQ bit was unchecked by xidachen@chromium.org
The CQ bit was checked by xidachen@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from junov@chromium.org Link to the patchset: https://codereview.chromium.org/2377883002/#ps60001 (title: "rebse after Blink Format")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Expose --disable-gpu-compositing browser arg to Blink Currently in Blink, there is no indication whether a compositor is GPU accelerated or software. This CL plumbs it to Blink, so that we can query whether --disable-gpu-compositing or --disable-gpu is set or not. BUG=650795 ========== to ========== Expose --disable-gpu-compositing browser arg to Blink Currently in Blink, there is no indication whether a compositor is GPU accelerated or software. This CL plumbs it to Blink, so that we can query whether --disable-gpu-compositing or --disable-gpu is set or not. BUG=650795 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Expose --disable-gpu-compositing browser arg to Blink Currently in Blink, there is no indication whether a compositor is GPU accelerated or software. This CL plumbs it to Blink, so that we can query whether --disable-gpu-compositing or --disable-gpu is set or not. BUG=650795 ========== to ========== Expose --disable-gpu-compositing browser arg to Blink Currently in Blink, there is no indication whether a compositor is GPU accelerated or software. This CL plumbs it to Blink, so that we can query whether --disable-gpu-compositing or --disable-gpu is set or not. BUG=650795 Committed: https://crrev.com/c234ff9ce6b4ce3c1561dc99ef30837d4000101e Cr-Commit-Position: refs/heads/master@{#422672} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/c234ff9ce6b4ce3c1561dc99ef30837d4000101e Cr-Commit-Position: refs/heads/master@{#422672}
Message was sent while issue was closed.
https://codereview.chromium.org/2377883002/diff/60001/content/child/runtime_f... File content/child/runtime_features.cc (right): https://codereview.chromium.org/2377883002/diff/60001/content/child/runtime_f... content/child/runtime_features.cc:119: command_line.HasSwitch(switches::kDisableGpu)) Sorry just noticed this but you don't need to check disable-gpu here. In that case we won't be able to make a context. Similarly we dont check for that flag here https://cs.chromium.org/chromium/src/content/renderer/render_thread_impl.cc?r... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
