Commit 49fa3eb5 authored by Mingjing Zhang's avatar Mingjing Zhang Committed by Commit Bot

Revert "Not disable acceleration in getImage if its Desynchronized"

This reverts commit edce90d5.

Reason for revert: crbug.com/1151440 Pixel_CanvasLowLatency2DImageData
test consistently fails on Android FYI Release builders, e.g.:

https://ci.chromium.org/p/chromium/builders/ci/Android%20FYI%20Release%20%28Nexus%205%29/b8863094711783933776

Original change's description:
> 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/+/2541063
> Reviewed-by: Aaron Krajeski <aaronhk@chromium.org>
> Commit-Queue: Juanmi Huertas <juanmihd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#829679}

TBR=fserb@chromium.org,aaronhk@chromium.org,juanmihd@chromium.org

Change-Id: I35417b2a319a610b7aec7123112db153cff3a806
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1112060
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2552924
Commit-Queue: Mingjing Zhang <mjzhang@chromium.org>
Reviewed-by: default avatarMingjing Zhang <mjzhang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829801}
parent 872df972
...@@ -1696,14 +1696,12 @@ ImageData* BaseRenderingContext2D::getImageDataInternal( ...@@ -1696,14 +1696,12 @@ 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.
// TODO(crbug.com/1090180): New Canvas2D API utilizes willReadFrequently // New Canvas2D API utilizes willReadFrequently attribute that let the users
// attribute that let the users indicate if a canvas will be read frequently // indicate if a canvas will be read frequently through getImageData, thus
// through getImageData, thus uses CPU rendering from the start in such cases. // uses CPU rendering from the start in such cases. (crbug.com/1090180)
if (!RuntimeEnabledFeatures::NewCanvas2DAPIEnabled()) { if (!RuntimeEnabledFeatures::NewCanvas2DAPIEnabled()) {
// GetImagedata is faster in Unaccelerated canvases. // GetImagedata is faster in Unaccelerated canvases
// In Desynchronized canvas disabling the acceleration will break if (IsAccelerated()) {
// putImageData: crbug.com/1112060.
if (IsAccelerated() && !IsDesynchronized()) {
DisableAcceleration(); DisableAcceleration();
base::UmaHistogramEnumeration("Blink.Canvas.GPUFallbackToCPU", base::UmaHistogramEnumeration("Blink.Canvas.GPUFallbackToCPU",
GPUFallbackToCPUScenario::kGetImageData); GPUFallbackToCPUScenario::kGetImageData);
......
...@@ -253,11 +253,6 @@ class MODULES_EXPORT BaseRenderingContext2D : public GarbageCollectedMixin, ...@@ -253,11 +253,6 @@ 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,9 +278,6 @@ class MODULES_EXPORT CanvasRenderingContext2D final ...@@ -278,9 +278,6 @@ 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,9 +122,6 @@ class MODULES_EXPORT OffscreenCanvasRenderingContext2D final ...@@ -122,9 +122,6 @@ 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