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

Issue 12210081: Paint low-res tiles without expensive filters. (Closed)

Created:
7 years, 10 months ago by Tom Hudson
Modified:
7 years, 10 months ago
Reviewers:
reed1, danakj, nduca, Sami, tomhudson
CC:
chromium-reviews, cc-bugs_chromium.org, reed1, andreip3000
Visibility:
Public.

Description

Paint low-res tiles without expensive filters. Creates the skia::PaintSimplifier class, which can do things like turn off antialiasing, blurring, or subpixel text rendering. Activates a PaintSimplifier in cc::TileManager::PerformRaster if we're in the middle of a fling or other raster-intense transient condition. BUG=174945 TBR=nduca,tomhudson CC=reed,skyostil,klobag

Patch Set 1 #

Total comments: 5

Patch Set 2 : Fix filter() return value bug, change low-res scale #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+104 lines, -2 lines) Patch
M cc/layer_tree_settings.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M cc/tile_manager.h View 2 chunks +3 lines, -0 lines 1 comment Download
M cc/tile_manager.cc View 7 chunks +24 lines, -1 line 3 comments Download
A skia/ext/paint_simplifier.h View 1 1 chunk +33 lines, -0 lines 1 comment Download
A skia/ext/paint_simplifier.cc View 1 1 chunk +41 lines, -0 lines 1 comment Download
M skia/skia.gyp View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
Tom Hudson
Here's a first implementation of the feature Grace asked for last night: turn off expensive ...
7 years, 10 months ago (2013-02-08 12:08:09 UTC) #1
Sami
Would it make sense to also couple the 16x => 4x low res change with ...
7 years, 10 months ago (2013-02-08 13:44:01 UTC) #2
Tom Hudson
On 2013/02/08 13:44:01, Sami wrote: > Would it make sense to also couple the 16x ...
7 years, 10 months ago (2013-02-08 13:47:26 UTC) #3
reed1
https://codereview.chromium.org/12210081/diff/1/skia/ext/paint_simplifier.cc File skia/ext/paint_simplifier.cc (right): https://codereview.chromium.org/12210081/diff/1/skia/ext/paint_simplifier.cc#newcode26 skia/ext/paint_simplifier.cc:26: //paint->setColor(SkColorSetRGB(255, 127, 127)); need to return true so that ...
7 years, 10 months ago (2013-02-08 14:15:26 UTC) #4
Tom Hudson
PTAL. This works. Sami and Andrei both liked the resultant quality. The homepage of theverge.com ...
7 years, 10 months ago (2013-02-08 14:47:39 UTC) #5
Sami
https://codereview.chromium.org/12210081/diff/9001/cc/tile_manager.cc File cc/tile_manager.cc (right): https://codereview.chromium.org/12210081/diff/9001/cc/tile_manager.cc#newcode76 cc/tile_manager.cc:76: Intentional change? https://codereview.chromium.org/12210081/diff/9001/cc/tile_manager.cc#newcode814 cc/tile_manager.cc:814: canvas.setDrawFilter(new skia::PaintSimplifier); Let's figure out ...
7 years, 10 months ago (2013-02-08 15:05:47 UTC) #6
reed1
lgtm https://codereview.chromium.org/12210081/diff/9001/skia/ext/paint_simplifier.cc File skia/ext/paint_simplifier.cc (right): https://codereview.chromium.org/12210081/diff/9001/skia/ext/paint_simplifier.cc#newcode27 skia/ext/paint_simplifier.cc:27: paint->setLCDRenderText(false); Did no-lcd in fact help? In the ...
7 years, 10 months ago (2013-02-08 15:07:10 UTC) #7
Sami
On 2013/02/08 15:07:10, reed1 wrote: > Did no-lcd in fact help? In the abstract, it ...
7 years, 10 months ago (2013-02-08 15:09:56 UTC) #8
reed1
ah, right you are (we do/should *not* use LCD on android). I was thinking desktop ...
7 years, 10 months ago (2013-02-08 15:31:11 UTC) #9
danakj
What is the desired behaviour when the scroll stops? Should the tiles that were painting ...
7 years, 10 months ago (2013-02-08 19:55:49 UTC) #10
klobag.chromium
On 2013/02/08 19:55:49, danakj wrote: > What is the desired behaviour when the scroll stops? ...
7 years, 10 months ago (2013-02-08 22:02:34 UTC) #11
danakj
On Fri, Feb 8, 2013 at 5:02 PM, <klobag@chromium.org> wrote: > On 2013/02/08 19:55:49, danakj ...
7 years, 10 months ago (2013-02-08 22:04:27 UTC) #12
enne (OOO)
I will reiterate danakj's concern, as I don't think it has been adequately answered. Low-res ...
7 years, 10 months ago (2013-02-09 07:11:18 UTC) #13
tomhudson
On 2013/02/09 07:11:18, enne wrote: > I will reiterate danakj's concern, as I don't think ...
7 years, 10 months ago (2013-02-09 08:36:36 UTC) #14
danakj
On 2013/02/09 08:36:36, tomhudson wrote: > On 2013/02/09 07:11:18, enne wrote: > > I will ...
7 years, 10 months ago (2013-02-09 18:50:23 UTC) #15
nduca
Reading the discussion here, I think the correct solution to this is a fully generic ...
7 years, 10 months ago (2013-02-13 07:04:37 UTC) #16
danakj
On Tue, Feb 12, 2013 at 11:04 PM, <nduca@chromium.org> wrote: > Reading the discussion here, ...
7 years, 10 months ago (2013-02-13 07:11:05 UTC) #17
nduca
Also , maybe we can separate out the paint simplifier patch from the cc parts ...
7 years, 10 months ago (2013-02-13 08:04:59 UTC) #18
Tom Hudson
7 years, 10 months ago (2013-02-13 19:44:18 UTC) #19
On 2013/02/13 08:04:59, nduca wrote:
> Also , maybe we can separate out the paint simplifier patch from the cc parts
of
> the patch. Sounds like we want to do something here, so we could start doing
> this in baby steps.

> Also , maybe we can separate out the paint simplifier patch from the cc parts
of
> the patch. Sounds like we want to do something here, so we could start doing
> this in baby steps.

Change to default low resolution in layer_tree_settings.cc is at
https://codereview.chromium.org/12258007/.
Paint simplifier is at https://codereview.chromium.org/12207157/.
I believe the MTV team is going to meet today/tomorrow to think about how to
best wire up the paint simplifier.

Powered by Google App Engine
This is Rietveld 408576698