Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(25)

Issue 562583002: Implement image-rendering:pixelated for accelerated 2D canvases. (Closed)

Created:
6 years, 3 months ago by jackhou1
Modified:
6 years ago
CC:
abarth-chromium, blink-reviews, blink-reviews-html_chromium.org, Rik, chrome-apps-syd-reviews_chromium.org, danakj, dglazkov+blink, krit, jbroman, pdr., rwlbuis, Stephen Chennney
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Implement image-rendering:pixelated for accelerated 2D canvases. This CL depends on a CC side change here: https://codereview.chromium.org/558083002/ BUG=134040, 424025, 317991 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187428

Patch Set 1 #

Total comments: 5

Patch Set 2 : Add tests. #

Total comments: 1

Patch Set 3 : Change setNearestNeighbor to setFilterLevel, use SkPaint::FilterLevel. #

Total comments: 1

Patch Set 4 : Set GL_NEAREST on the texture, invalidate the ImageBuffer and reconstruct the TextureMailbox when f… #

Total comments: 4

Patch Set 5 : Add nearestNeighbor to WebExternalTextureMailbox. #

Patch Set 6 : Add a test for drawing canvas to canvas. #

Patch Set 7 : Change to WebExternalTextureLayer::setNearestNeighbor, address comments. #

Patch Set 8 : Remove printfs, update test comments. #

Total comments: 10

Patch Set 9 : Sync and rebase #

Patch Set 10 : Address comments #

Patch Set 11 : Sync and rebase, fix up tests, don't invalidate surface when changing filter (in Canvas2DLayerBridg… #

Patch Set 12 : Make tests smaller. #

Total comments: 3

Patch Set 13 : Address comments #

Patch Set 14 : Sync and rebase #

Patch Set 15 : Fix and add test for off-screen canvas case. #

Patch Set 16 : Use renderStyle instead of computedStyle. #

Patch Set 17 : #include NodeRenderStyle.h #

Total comments: 6

Patch Set 18 : Sync and rebase #

Patch Set 19 : Address comments #

Patch Set 20 : updateRenderTreeIfNeeded in createImageBufferInternal #

Patch Set 21 : Sync and rebase #

Total comments: 5

Patch Set 22 : Sync and rebase #

Patch Set 23 : Remove setFilterLevel in createRenderer. #

Patch Set 24 : Sync and rebase. Remove test expectations. #

Total comments: 6

Patch Set 25 : Address comments #

Patch Set 26 : Sync and rebase #

Patch Set 27 : setFilterLevel in createImageBufferInternal #

Patch Set 28 : Sync and rebase #

Total comments: 2

Patch Set 29 : Fix crash on Linux/Win. #

Patch Set 30 : Don't call setImageInterpolationQuality #

Unified diffs Side-by-side diffs Delta from patch set Stats (+164 lines, -17 lines) Patch
A LayoutTests/fast/canvas/pixelated.html View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +45 lines, -0 lines 0 comments Download
A LayoutTests/fast/canvas/pixelated-canvas.html View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +36 lines, -0 lines 0 comments Download
A + LayoutTests/fast/canvas/pixelated-canvas-expected.png View 1 2 3 4 5 6 7 8 9 10 11 Binary file 0 comments Download
A + LayoutTests/fast/canvas/pixelated-canvas-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 0 chunks +-1 lines, --1 lines 0 comments Download
A LayoutTests/fast/canvas/pixelated-expected.png View 1 2 3 4 5 6 7 8 9 10 11 Binary file 0 comments Download
A + LayoutTests/fast/canvas/pixelated-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/canvas/pixelated-off-screen.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +8 lines, -8 lines 0 comments Download
A + LayoutTests/fast/canvas/pixelated-off-screen-expected.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 3 chunks +6 lines, -9 lines 0 comments Download
A LayoutTests/fast/canvas/pixelated-resize.html View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +37 lines, -0 lines 0 comments Download
A + LayoutTests/fast/canvas/pixelated-resize-expected.png View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 Binary file 0 comments Download
A + LayoutTests/fast/canvas/pixelated-resize-expected.txt View 1 2 3 4 5 6 7 8 9 0 chunks +-1 lines, --1 lines 0 comments Download
A LayoutTests/virtual/gpu/fast/canvas/pixelated-expected.png View 1 2 3 4 5 6 7 8 9 10 11 Binary file 0 comments Download
A + LayoutTests/virtual/gpu/fast/canvas/pixelated-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/core/html/HTMLCanvasElement.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 3 chunks +9 lines, -1 line 0 comments Download
M Source/platform/graphics/Canvas2DImageBufferSurface.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M Source/platform/graphics/Canvas2DLayerBridge.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 3 chunks +3 lines, -0 lines 0 comments Download
M Source/platform/graphics/Canvas2DLayerBridge.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 6 chunks +17 lines, -3 lines 0 comments Download
M Source/platform/graphics/ImageBuffer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 3 chunks +4 lines, -0 lines 0 comments Download
M Source/platform/graphics/ImageBufferSurface.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 55 (7 generated)
jackhou1
junov, would you mind giving me some advice on this? I'm not very familiar with ...
6 years, 3 months ago (2014-09-10 08:20:18 UTC) #2
Justin Novosad
I think you are on the right track, but you need to write layout tests ...
6 years, 3 months ago (2014-09-10 14:28:31 UTC) #3
Stephen White
https://codereview.chromium.org/562583002/diff/1/Source/platform/graphics/ImageBuffer.h File Source/platform/graphics/ImageBuffer.h (right): https://codereview.chromium.org/562583002/diff/1/Source/platform/graphics/ImageBuffer.h#newcode86 Source/platform/graphics/ImageBuffer.h:86: void setNearestNeighbor(bool nearestNeighbor) { m_surface->setNearestNeighbor(nearestNeighbor); } On 2014/09/10 14:28:31, ...
6 years, 3 months ago (2014-09-10 15:37:09 UTC) #5
Justin Novosad
On 2014/09/10 15:37:09, Stephen White wrote: > https://codereview.chromium.org/562583002/diff/1/Source/platform/graphics/ImageBuffer.h > File Source/platform/graphics/ImageBuffer.h (right): > > https://codereview.chromium.org/562583002/diff/1/Source/platform/graphics/ImageBuffer.h#newcode86 ...
6 years, 3 months ago (2014-09-10 17:46:48 UTC) #6
jackhou1
> I think you are on the right track, but you need to write layout ...
6 years, 3 months ago (2014-09-11 08:26:32 UTC) #7
Justin Novosad
On 2014/09/11 08:26:32, jackhou1 wrote: > > I think you are on the right track, ...
6 years, 3 months ago (2014-09-11 15:20:10 UTC) #8
Justin Novosad
https://codereview.chromium.org/562583002/diff/40001/Source/core/html/HTMLCanvasElement.cpp File Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/562583002/diff/40001/Source/core/html/HTMLCanvasElement.cpp#newcode125 Source/core/html/HTMLCanvasElement.cpp:125: m_imageBuffer->setFilterLevel(computedStyle()->imageRendering() == ImageRenderingPixelated ? SkPaint::kNone_FilterLevel : SkPaint::kLow_FilterLevel); You need ...
6 years, 3 months ago (2014-09-11 15:42:30 UTC) #9
Stephen White
On 2014/09/10 17:46:48, junov wrote: > On 2014/09/10 15:37:09, Stephen White wrote: > > > ...
6 years, 3 months ago (2014-09-11 16:07:41 UTC) #10
jamesr
On 2014/09/11 16:07:41, Stephen White wrote: > On 2014/09/10 17:46:48, junov wrote: > > On ...
6 years, 3 months ago (2014-09-11 17:28:19 UTC) #11
Justin Novosad
On 2014/09/11 17:28:19, jamesr wrote: > > On another topic, don't we need to control ...
6 years, 3 months ago (2014-09-11 17:40:28 UTC) #12
jackhou1
Replying to try to catch MTV time. I'll work on this when I'm in the ...
6 years, 3 months ago (2014-09-11 18:16:58 UTC) #13
Justin Novosad
On 2014/09/11 18:16:58, jackhou1 wrote: > Replying to try to catch MTV time. I'll work ...
6 years, 3 months ago (2014-09-11 18:30:13 UTC) #14
Stephen White
On 2014/09/11 17:28:19, jamesr wrote: > On 2014/09/11 16:07:41, Stephen White wrote: > > On ...
6 years, 3 months ago (2014-09-11 20:43:23 UTC) #15
jackhou1
Okay, I've updated this CL to match Patch Set 3 on the compositor CL: https://codereview.chromium.org/558083002/ ...
6 years, 1 month ago (2014-10-28 03:33:15 UTC) #16
Justin Novosad
What hapens if we do a canvas to canvas copy (drawImage), where the source canvas ...
6 years, 1 month ago (2014-10-28 20:27:44 UTC) #17
jackhou1
> What hapens if we do a canvas to canvas copy (drawImage), where the source ...
6 years, 1 month ago (2014-10-29 00:56:39 UTC) #18
Justin Novosad
> > Without this change, a pixelated canvas is intially drawn properly, but when I ...
6 years, 1 month ago (2014-10-29 16:32:58 UTC) #19
jackhou1
PTAL The cc CL is almost done. One remaining comment is to only pass a ...
6 years, 1 month ago (2014-11-13 05:57:46 UTC) #20
jackhou1
On 2014/11/13 05:57:46, jackhou1 wrote: > PTAL > > The cc CL is almost done. ...
6 years, 1 month ago (2014-11-13 05:59:20 UTC) #21
jackhou1
junov, ping.
6 years, 1 month ago (2014-11-17 05:06:05 UTC) #22
Justin Novosad
https://codereview.chromium.org/562583002/diff/140001/LayoutTests/fast/canvas/pixelated-canvas.html File LayoutTests/fast/canvas/pixelated-canvas.html (right): https://codereview.chromium.org/562583002/diff/140001/LayoutTests/fast/canvas/pixelated-canvas.html#newcode8 LayoutTests/fast/canvas/pixelated-canvas.html:8: height: 1000px; Too large for layout tests. https://codereview.chromium.org/562583002/diff/140001/LayoutTests/fast/canvas/pixelated.html File ...
6 years, 1 month ago (2014-11-18 16:32:50 UTC) #23
jackhou1
I've updated the tests so that they use an existing image. (Sorry for not uploading ...
6 years ago (2014-12-01 07:00:09 UTC) #25
Stephen White
https://codereview.chromium.org/562583002/diff/140001/LayoutTests/fast/canvas/pixelated.html File LayoutTests/fast/canvas/pixelated.html (right): https://codereview.chromium.org/562583002/diff/140001/LayoutTests/fast/canvas/pixelated.html#newcode7 LayoutTests/fast/canvas/pixelated.html:7: width: 1000px; On 2014/12/01 07:00:08, jackhou1 wrote: > On ...
6 years ago (2014-12-01 15:53:48 UTC) #26
jackhou1
https://codereview.chromium.org/562583002/diff/140001/LayoutTests/fast/canvas/pixelated.html File LayoutTests/fast/canvas/pixelated.html (right): https://codereview.chromium.org/562583002/diff/140001/LayoutTests/fast/canvas/pixelated.html#newcode7 LayoutTests/fast/canvas/pixelated.html:7: width: 1000px; On 2014/12/01 15:53:48, Stephen White wrote: > ...
6 years ago (2014-12-02 00:50:48 UTC) #27
Justin Novosad
LGTM with small correction. https://codereview.chromium.org/562583002/diff/240001/Source/platform/graphics/Canvas2DLayerBridge.h File Source/platform/graphics/Canvas2DLayerBridge.h (right): https://codereview.chromium.org/562583002/diff/240001/Source/platform/graphics/Canvas2DLayerBridge.h#newcode143 Source/platform/graphics/Canvas2DLayerBridge.h:143: GLenum m_lastFilter; It looks like ...
6 years ago (2014-12-02 14:56:35 UTC) #28
Justin Novosad
https://codereview.chromium.org/562583002/diff/240001/Source/platform/graphics/Canvas2DLayerBridge.cpp File Source/platform/graphics/Canvas2DLayerBridge.cpp (right): https://codereview.chromium.org/562583002/diff/240001/Source/platform/graphics/Canvas2DLayerBridge.cpp#newcode434 Source/platform/graphics/Canvas2DLayerBridge.cpp:434: if (image->uniqueID() == m_lastImageId && filter == m_lastFilter) I ...
6 years ago (2014-12-02 16:47:20 UTC) #29
jackhou1
https://codereview.chromium.org/562583002/diff/240001/Source/platform/graphics/Canvas2DLayerBridge.cpp File Source/platform/graphics/Canvas2DLayerBridge.cpp (right): https://codereview.chromium.org/562583002/diff/240001/Source/platform/graphics/Canvas2DLayerBridge.cpp#newcode434 Source/platform/graphics/Canvas2DLayerBridge.cpp:434: if (image->uniqueID() == m_lastImageId && filter == m_lastFilter) On ...
6 years ago (2014-12-03 03:16:34 UTC) #30
jackhou1
tkent, please review for OWNERS in public/platform/
6 years ago (2014-12-03 06:53:41 UTC) #32
tkent
public/platform/ lgtm
6 years ago (2014-12-03 07:01:30 UTC) #33
jackhou1
junov, PTAL There was a bug where if you create a canvas then attach it ...
6 years ago (2014-12-04 05:37:58 UTC) #34
Justin Novosad
There are some layout test failures with display_list_2d_canvas (see win_blink_rel bot results). You do not ...
6 years ago (2014-12-04 15:27:07 UTC) #35
Justin Novosad
On 2014/12/04 15:27:07, junov wrote: > We should always use the computedStyle. To clarify why ...
6 years ago (2014-12-04 15:46:45 UTC) #36
jackhou1
> There are some layout test failures with display_list_2d_canvas (see > win_blink_rel bot results). You ...
6 years ago (2014-12-05 02:00:58 UTC) #37
jackhou1
Adding a document().updateRenderTreeIfNeeded() seems to have fixed the crash. Is that the appropriate thing to ...
6 years ago (2014-12-05 06:58:42 UTC) #38
jackhou1
junov, PTAL I just want to check that calling document().updateRenderTreeIfNeeded() in createImageBufferInternal is okay. On ...
6 years ago (2014-12-09 00:23:03 UTC) #39
Justin Novosad
I would like someone more familiar with the intricacies of style recalc to look at ...
6 years ago (2014-12-09 15:17:01 UTC) #41
jackhou1
esprehn, ping
6 years ago (2014-12-12 06:02:29 UTC) #42
Justin Novosad
https://codereview.chromium.org/562583002/diff/420001/LayoutTests/TestExpectations File LayoutTests/TestExpectations (right): https://codereview.chromium.org/562583002/diff/420001/LayoutTests/TestExpectations#newcode1611 LayoutTests/TestExpectations:1611: crbug.com/439251 virtual/display_list_2d_canvas/fast/canvas/pixelated-resize.html [ ImageOnlyFailure ] You can remove these ...
6 years ago (2014-12-15 19:38:54 UTC) #43
Justin Novosad
+haraken +jchaffraix or esprehn Can one of you confirm whether the new call to document().updateRenderTreeIfNeeded(); ...
6 years ago (2014-12-15 20:00:08 UTC) #45
esprehn
https://codereview.chromium.org/562583002/diff/420001/Source/core/html/HTMLCanvasElement.cpp File Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/562583002/diff/420001/Source/core/html/HTMLCanvasElement.cpp#newcode143 Source/core/html/HTMLCanvasElement.cpp:143: m_imageBuffer->setFilterLevel(style->imageRendering() == ImageRenderingPixelated ? SkPaint::kNone_FilterLevel : SkPaint::kLow_FilterLevel); If you ...
6 years ago (2014-12-15 20:24:51 UTC) #46
jackhou1
https://codereview.chromium.org/562583002/diff/420001/Source/core/html/HTMLCanvasElement.cpp File Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/562583002/diff/420001/Source/core/html/HTMLCanvasElement.cpp#newcode143 Source/core/html/HTMLCanvasElement.cpp:143: m_imageBuffer->setFilterLevel(style->imageRendering() == ImageRenderingPixelated ? SkPaint::kNone_FilterLevel : SkPaint::kLow_FilterLevel); On 2014/12/15 ...
6 years ago (2014-12-15 23:17:24 UTC) #47
Justin Novosad
Great! Let's get this committed! LGTM
6 years ago (2014-12-16 13:05:23 UTC) #48
esprehn
https://codereview.chromium.org/562583002/diff/470001/Source/core/html/HTMLCanvasElement.cpp File Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/562583002/diff/470001/Source/core/html/HTMLCanvasElement.cpp#newcode635 Source/core/html/HTMLCanvasElement.cpp:635: m_imageBuffer->setFilterLevel(pixelated ? SkPaint::kNone_FilterLevel : SkPaint::kLow_FilterLevel); You don't need this ...
6 years ago (2014-12-16 19:22:33 UTC) #49
jackhou1
https://codereview.chromium.org/562583002/diff/470001/Source/core/html/HTMLCanvasElement.cpp File Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/562583002/diff/470001/Source/core/html/HTMLCanvasElement.cpp#newcode635 Source/core/html/HTMLCanvasElement.cpp:635: m_imageBuffer->setFilterLevel(pixelated ? SkPaint::kNone_FilterLevel : SkPaint::kLow_FilterLevel); On 2014/12/16 19:22:32, esprehn ...
6 years ago (2014-12-17 01:28:10 UTC) #50
esprehn
lgtm, yeah it's because the style is not dirty. This seems fine, the code duplication ...
6 years ago (2014-12-17 23:05:19 UTC) #51
jackhou1
https://codereview.chromium.org/562583002/diff/550001/Source/core/html/HTMLCanvasElement.cpp File Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/562583002/diff/550001/Source/core/html/HTMLCanvasElement.cpp#newcode159 Source/core/html/HTMLCanvasElement.cpp:159: m_imageBuffer->context()->setImageInterpolationQuality(pixelated ? InterpolationLow : CanvasDefaultInterpolationQuality); On 2014/12/17 23:05:19, esprehn ...
6 years ago (2014-12-18 03:48:08 UTC) #52
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/562583002/590001
6 years ago (2014-12-18 03:49:27 UTC) #54
commit-bot: I haz the power
6 years ago (2014-12-18 03:53:35 UTC) #55
Message was sent while issue was closed.
Committed patchset #30 (id:590001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187428

Powered by Google App Engine
This is Rietveld 408576698