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

Issue 2674003002: Scalable spelling/grammar markers (Closed)

Created:
3 years, 10 months ago by f(malita)
Modified:
3 years, 9 months ago
CC:
chromium-reviews, krit, drott+blinkwatch_chromium.org, blink-reviews-platform-graphics_chromium.org, Stephen Chennney, dshwang, pdr+graphicswatchlist_chromium.org, jbroman, Justin Novosad, Rik, f(malita), blink-reviews, kinuko+watch, ajuma+watch_chromium.org, danakj+watch_chromium.org, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Scalable spelling/grammar markers Replace the inlined bitmap markers with vector drawings, to improve rendering at arbitrary zoom levels. Use cached SKP recordings for marker fragments, and a picture shader for tiling in X. In order to minimize disruption, this change aims to keep rendering as close to the original bitmaps as possible. This can be revisited in the future (to e.g. improve dot spacing/distribution on Mac). Performance should be comparable to the current bitmap shader impl: Skia caches picture shader tiles, so for a given zoom level all markers should be hitting the same pre-rasterized cache entry (and essentially devolve into bitmap shaders). R=schenney@chromium.org BUG=686792 Review-Url: https://codereview.chromium.org/2674003002 Cr-Commit-Position: refs/heads/master@{#449024} Committed: https://chromium.googlesource.com/chromium/src/+/b09cbee7a1edeb61ea0a594ac800090f62cb4248

Patch Set 1 #

Patch Set 2 : WIP #

Patch Set 3 : cleanup #

Patch Set 4 : WIP #

Patch Set 5 : cleanup #

Patch Set 6 : formatting #

Patch Set 7 : fix dsf handling #

Patch Set 8 : exclude origin offset from local matrix #

Patch Set 9 : adjust gradient pts #

Patch Set 10 : adjust Mac colors #

Patch Set 11 : rebaseline #

Patch Set 12 : formatting #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+115 lines, -212 lines) Patch
M third_party/WebKit/LayoutTests/platform/linux/editing/spelling/grammar-markers-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/editing/spelling/grammar-markers-hidpi-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/editing/spelling/inline-spelling-markers-hidpi-composited-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/editing/spelling/inline-spelling-markers-hidpi-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/editing/spelling/inline_spelling_markers-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/paint/spellmarkers/document-markers-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/paint/spellmarkers/document-markers-zoom-125-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/paint/spellmarkers/document-markers-zoom-150-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/paint/spellmarkers/document-markers-zoom-175-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/paint/spellmarkers/document-markers-zoom-200-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/paint/spellmarkers/document-markers-zoom-250-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/editing/spelling/grammar-markers-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/editing/spelling/grammar-markers-hidpi-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/editing/spelling/inline-spelling-markers-hidpi-composited-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/editing/spelling/inline-spelling-markers-hidpi-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/editing/spelling/inline_spelling_markers-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/paint/spellmarkers/document-markers-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/paint/spellmarkers/document-markers-zoom-125-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/paint/spellmarkers/document-markers-zoom-150-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/paint/spellmarkers/document-markers-zoom-175-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/paint/spellmarkers/document-markers-zoom-200-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/paint/spellmarkers/document-markers-zoom-250-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/editing/spelling/grammar-markers-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/editing/spelling/grammar-markers-hidpi-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/editing/spelling/inline-spelling-markers-hidpi-composited-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/editing/spelling/inline-spelling-markers-hidpi-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/editing/spelling/inline_spelling_markers-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/paint/spellmarkers/document-markers-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/paint/spellmarkers/document-markers-zoom-125-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/paint/spellmarkers/document-markers-zoom-150-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/paint/spellmarkers/document-markers-zoom-175-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/paint/spellmarkers/document-markers-zoom-200-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/paint/spellmarkers/document-markers-zoom-250-expected.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M third_party/WebKit/Source/platform/graphics/GraphicsContext.h View 1 2 2 chunks +0 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +115 lines, -206 lines 2 comments Download

Dependent Patchsets:

Messages

Total messages: 40 (32 generated)
f(malita)
Diffs show the expected hi-res improvements - Mac: https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel_ng/383353/layout-test-results/results.html Win/Linux: https://storage.googleapis.com/chromium-layout-test-archives/win10_blink_rel/1964/layout-test-results/results.html
3 years, 10 months ago (2017-02-07 20:43:32 UTC) #22
Stephen Chennney
LGTM. Thanks for following up.
3 years, 10 months ago (2017-02-07 21:12:02 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2674003002/200001
3 years, 10 months ago (2017-02-08 14:55:45 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2674003002/220001
3 years, 10 months ago (2017-02-08 15:09:18 UTC) #32
commit-bot: I haz the power
Committed patchset #12 (id:220001) as https://chromium.googlesource.com/chromium/src/+/b09cbee7a1edeb61ea0a594ac800090f62cb4248
3 years, 10 months ago (2017-02-08 17:30:57 UTC) #36
Stephen White
Just saw this in the Skia update. This is really awesome work, Florin!
3 years, 9 months ago (2017-03-02 15:04:00 UTC) #37
Stephen White
https://codereview.chromium.org/2674003002/diff/220001/third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp File third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp (right): https://codereview.chromium.org/2674003002/diff/220001/third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp#newcode47 third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp:47: #include "third_party/skia/include/core/SkUnPreMultiply.h" uNit: is this still used anywhere? Maybe ...
3 years, 9 months ago (2017-03-02 15:06:26 UTC) #39
f(malita)
3 years, 9 months ago (2017-03-03 19:16:09 UTC) #40
Message was sent while issue was closed.
https://codereview.chromium.org/2674003002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp (right):

https://codereview.chromium.org/2674003002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp:47: #include
"third_party/skia/include/core/SkUnPreMultiply.h"
On 2017/03/02 15:06:26, Stephen White wrote:
> uNit: is this still used anywhere? Maybe it could be removed in a followup if
> not.

Hmm, nope, how did it sneak in there? :)

Thanks, I'll remove.

Powered by Google App Engine
This is Rietveld 408576698