New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 792110 link

Starred by 2 users
Status: Fixed
Owner:
█ zakerinasab@chromium.org
Last visit > 30 days ago
Closed: Dec 2017
Cc:
█ junov@chromium.org
fs...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Remove ImageData from toBlob/toDataURL code path

Project Member Reported by zakerinasab@chromium.org, Dec 5 2017 
After https://chromium-review.googlesource.com/c/chromium/src/+/780279/ is landed, we are using both StaticBitmapImage and ImageData in HTMLCanvasElement toBlob/toDataURL code path. By removing ImageData usage from the code path, we clean the code and pave the way for adding color management to toBlob and toDataURL.
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67

commit baf563db4c5ca4eb350b96a3c7dc16b697ae3c67
Author: Reza.Zakerinasab <zakerinasab@chromium.org>
Date: Wed Dec 06 23:30:39 2017

Remove ImageData from toBlob/toDataURL code path

This change removes ImageData usage from toBlob/toDataURL code path
from HTMLCanvasElement calls down to the CanvasAsyncBlobCreator.

Bug:  792110 ,  657870 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I3e222a68767c1a476f09c1b6188699f62449b95a
Reviewed-on: https://chromium-review.googlesource.com/809097
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Olivia Lai <xlai@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522248}
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/core/html/HTMLCanvasElement.h
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.h
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreatorTest.cpp
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.h
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/modules/canvas/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/modules/canvas/offscreencanvas2d/OffscreenCanvasRenderingContext2D.h
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.cpp
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.h
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/baf563db4c5ca4eb350b96a3c7dc16b697ae3c67/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h

Comment 2 by zakerinasab@chromium.org, Dec 7 2017

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7

commit 78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7
Author: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Date: Mon Dec 11 17:56:35 2017

Revert "Remove ImageData from toBlob/toDataURL code path"

This reverts commit baf563db4c5ca4eb350b96a3c7dc16b697ae3c67.

Reason for revert: Crash in SkPngEncoder::onEncodeRows ( crbug.com/793571 )

Original change's description:
> Remove ImageData from toBlob/toDataURL code path
> 
> This change removes ImageData usage from toBlob/toDataURL code path
> from HTMLCanvasElement calls down to the CanvasAsyncBlobCreator.
> 
> Bug:  792110 ,  657870 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: I3e222a68767c1a476f09c1b6188699f62449b95a
> Reviewed-on: https://chromium-review.googlesource.com/809097
> Reviewed-by: Justin Novosad <junov@chromium.org>
> Reviewed-by: Olivia Lai <xlai@chromium.org>
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#522248}

TBR=kbr@chromium.org,junov@chromium.org,fserb@chromium.org,xlai@chromium.org,zakerinasab@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  792110 ,  657870 
Change-Id: Ie3e35ab6149a1602ee023f01e8e2eea0dd1b2208
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/819491
Reviewed-by: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523132}
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/core/html/HTMLCanvasElement.h
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.h
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreatorTest.cpp
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.h
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/modules/canvas/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/modules/canvas/offscreencanvas2d/OffscreenCanvasRenderingContext2D.h
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.cpp
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.h
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/78ab7b0cc9d04018e0b24c054c28a49dd4f68cb7/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 13 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1893591e9688033abd956a1f4195568afabf4408

commit 1893591e9688033abd956a1f4195568afabf4408
Author: Reza.Zakerinasab <zakerinasab@chromium.org>
Date: Wed Dec 13 18:36:03 2017

Reland Remove ImageData from toBlob/toDataURL code path

The original change removed ImageData usage from toBlob/toDataURL code path
from HTMLCanvasElement calls down to the CanvasAsyncBlobCreator. It was
reverted after crashing on canvases with more than 16M pixels.

This change relands the CL. The problem with big canvases was that there
is a logic in WebGLRenderingContextBase that down scales the DrawingBuffer
if it has more than 16M pixles, The previous code was using the size asked
by user instead of the size adjusted in WebGLRenderingContextBase, leading
to a crash down the path while encoding ImageDataBuffer.

Patch set 1 is the reverted CL (chromium-review.googlesource.com/c/chromium/src/+/809097)

TBR=kbr@chromium.org

Bug:  792110 ,  657870 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I7ab2d6fa8359e3adb3bfa13cc24783a1072eece6
Reviewed-on: https://chromium-review.googlesource.com/822910
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523832}
[add] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-crash.html
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/core/html/HTMLCanvasElement.h
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.h
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreatorTest.cpp
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.h
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/modules/canvas/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/modules/canvas/offscreencanvas2d/OffscreenCanvasRenderingContext2D.h
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.cpp
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.h
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/1893591e9688033abd956a1f4195568afabf4408/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h
►

Sign in to add a comment