Commit edce90d5 authored by Juanmi Huertas's avatar Juanmi Huertas Committed by Commit Bot

Not disable acceleration in getImage if its Desynchronized

This is a temporary fix for this issue, that will totally be fixed
after we land the new 2d canvas api (that will completely remove the
path of code causing this regression in base_rendering_context_2d.cc:
  if (!RuntimeEnabledFeatures::NewCanvas2DAPIEnabled()) {
    // GetImagedata is faster in Unaccelerated canvases
    // Do not disaccelerate if Desync2d Canvas on android.
    // Issue 1112060: putImageData() does not work for desynchronized 2D canvas
    // on Android
    if (IsAccelerated()) {
      DisableAcceleration();
      base::UmaHistogramEnumeration("Blink.Canvas.GPUFallbackToCPU",
                                    GPUFallbackToCPUScenario::kGetImageData);
    }
  }

This CL specifically disables that path if the canvas is Desynchronized.
To do so this CL adds some infrastructural elements to be able to access
to the desynchronization setting inside getImage.

This issue is only happening in a very specific corner case, which
proves hard to be tested. This code will be deleted once we enable
the new canvas api - so this is a temporary fix.

Bug: 1112060
Change-Id: I60b90f716332b3969c82dfef3431e4cbd66e6500
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2541063Reviewed-by: default avatarAaron Krajeski <aaronhk@chromium.org>
Commit-Queue: Juanmi Huertas <juanmihd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829679}
parent 75fea71b
...@@ -1696,12 +1696,14 @@ ImageData* BaseRenderingContext2D::getImageDataInternal( ...@@ -1696,12 +1696,14 @@ ImageData* BaseRenderingContext2D::getImageDataInternal(
// TODO(crbug.com/1101055): Remove the check for NewCanvas2DAPI flag once // TODO(crbug.com/1101055): Remove the check for NewCanvas2DAPI flag once
// released. // released.
// New Canvas2D API utilizes willReadFrequently attribute that let the users // TODO(crbug.com/1090180): New Canvas2D API utilizes willReadFrequently
// indicate if a canvas will be read frequently through getImageData, thus // attribute that let the users indicate if a canvas will be read frequently
// uses CPU rendering from the start in such cases. (crbug.com/1090180) // through getImageData, thus uses CPU rendering from the start in such cases.
if (!RuntimeEnabledFeatures::NewCanvas2DAPIEnabled()) { if (!RuntimeEnabledFeatures::NewCanvas2DAPIEnabled()) {
// GetImagedata is faster in Unaccelerated canvases // GetImagedata is faster in Unaccelerated canvases.
if (IsAccelerated()) { // In Desynchronized canvas disabling the acceleration will break
// putImageData: crbug.com/1112060.
if (IsAccelerated() && !IsDesynchronized()) {
DisableAcceleration(); DisableAcceleration();
base::UmaHistogramEnumeration("Blink.Canvas.GPUFallbackToCPU", base::UmaHistogramEnumeration("Blink.Canvas.GPUFallbackToCPU",
GPUFallbackToCPUScenario::kGetImageData); GPUFallbackToCPUScenario::kGetImageData);
......
...@@ -253,6 +253,11 @@ class MODULES_EXPORT BaseRenderingContext2D : public GarbageCollectedMixin, ...@@ -253,6 +253,11 @@ class MODULES_EXPORT BaseRenderingContext2D : public GarbageCollectedMixin,
virtual bool HasAlpha() const = 0; virtual bool HasAlpha() const = 0;
virtual bool IsDesynchronized() const {
NOTREACHED();
return false;
}
virtual bool isContextLost() const = 0; virtual bool isContextLost() const = 0;
virtual void WillDrawImage(CanvasImageSource*) const {} virtual void WillDrawImage(CanvasImageSource*) const {}
......
...@@ -278,6 +278,9 @@ class MODULES_EXPORT CanvasRenderingContext2D final ...@@ -278,6 +278,9 @@ class MODULES_EXPORT CanvasRenderingContext2D final
bool IsAccelerated() const override; bool IsAccelerated() const override;
bool IsOriginTopLeft() const override; bool IsOriginTopLeft() const override;
bool HasAlpha() const override { return CreationAttributes().alpha; } bool HasAlpha() const override { return CreationAttributes().alpha; }
bool IsDesynchronized() const override {
return CreationAttributes().desynchronized;
}
void SetIsInHiddenPage(bool) override; void SetIsInHiddenPage(bool) override;
void SetIsBeingDisplayed(bool) override; void SetIsBeingDisplayed(bool) override;
void Stop() final; void Stop() final;
......
...@@ -122,6 +122,9 @@ class MODULES_EXPORT OffscreenCanvasRenderingContext2D final ...@@ -122,6 +122,9 @@ class MODULES_EXPORT OffscreenCanvasRenderingContext2D final
void ValidateStateStackWithCanvas(const cc::PaintCanvas*) const final; void ValidateStateStackWithCanvas(const cc::PaintCanvas*) const final;
bool HasAlpha() const final { return CreationAttributes().alpha; } bool HasAlpha() const final { return CreationAttributes().alpha; }
bool IsDesynchronized() const final {
return CreationAttributes().desynchronized;
}
bool isContextLost() const override; bool isContextLost() const override;
ImageBitmap* TransferToImageBitmap(ScriptState*) final; ImageBitmap* TransferToImageBitmap(ScriptState*) final;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment