588271 - Improve border rendering code for running on GPUs

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

We currently do some elaborate work to be faster on software, this probably makes us slower on hardware. We should add a version for GPUs.

Comment 1

[Bas Schouten (:bas.schouten)]

As Jeff has rightfully been asking for some more concrete numbers here, I'm adding some profiling here, this is from resizing a window showing 'about:blank' i.e. rendering our UI, painting time is distributed as follows:

Process, Callees, Module, Function, Weight, % Weight, Count, TimeStamp
firefox.exe (4292), xul.dll!mozilla::FrameLayerBuilder::DrawThebesLayer, , , 7172.555024, 4.91, 7169, 
,    |- xul.dll!nsDisplayBorder::Paint, , , 3975.897323, 2.72, 3975, 
,    |- xul.dll!nsDisplayBackground::Paint, , , 1103.336155, 0.76, 1103, 
,    |- xul.dll!mozilla::layers::BasicLayerManager::EndTransaction, , , 578.951741, 0.40, 578, 
,    |- xul.dll!nsDisplayXULTextBox::Paint, , , 512.074132, 0.35, 512, 
,    |- xul.dll!nsDisplaySVGEffects::Paint, , , 378.912356, 0.26, 378, 
,    |- xul.dll!nsDisplayXULImage::Paint, , , 197.126124, 0.13, 197, 
,    |- xul.dll!nsDisplayBoxShadowOuter::Paint, , , 193.154272, 0.13, 193, 
,    |- xul.dll!nsDisplayBoxShadowInner::Paint, , , 183.042885, 0.13, 183, 
,    |- xul.dll!nsDisplayItem::RecomputeVisibility, , , 10.007181, 0.01, 10, 
,    |- xul.dll!nsDisplayCaret::Paint, , , 8.999948, 0.01, 9, 
,    |- xul.dll!nsDisplayText::Paint, , , 8.999565, 0.01, 9, 
,    |- xul.dll!mozilla::FrameLayerBuilder::DrawThebesLayer<itself>, xul.dll, mozilla::FrameLayerBuilder::DrawThebesLayer, 8.026816, 0.01, 8, 

i.e. more than 50% of the time painting is spent inside border drawing code.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Some quick ideas, any/all of which are likely to help:

disabling add in some cases:
- disable OPERATOR_ADD optimization for chrome
- disable OPERATOR_ADD optimization for 1px borders (maybe only in chrome -- has an issue in that the pixel right at the join of two edges can still look noticeably wrong)

not needing to use add:
- implement gradients for corners: set up a linear gradient between the source and dest color and stroke with that, instead of doing the pushgroup/operator add dance -- this is a good thing to do regardless and would be quite easy to implement, but hard to get right if you have two different joining border styles.  Different widths is probably possible, if you compute the outline and fill; same with solid-double etc.  But dotted/solid join would be a challenge, but that is so rare that maybe we don't care that it can look wrong (just use OVER in that case with a clip).

Comment 3

[Bas Schouten (:bas.schouten)]

I've concluded a large part of the problem here is not even so much the usage of OPERATOR_ADD, but also the extensive usage of geometric clipping. This should be avoidable a lot.

Comment 4

[Bas Schouten (:bas.schouten)]

I've been working on special D2D border rendering code that avoids using any clips and compositions. Just strokes and fill. At this point it's still in the early stages but performance wise it seems to be working out. I've gotten some great performance but I'm currently starting to work on (for the simple, solid border cases still), using gradients for border color transitions. The problem is that gradients are still fairly expensive in D2D (note the gradient code, in a very preliminary first estimate, still seems 4x+ faster for our UI (which is mainly rounded rectangles with different colors) than the current code). I'd really be interested if anyone has any other ideas.

Comment 5

[Bas Schouten (:bas.schouten)]

Attachment: First code on fast borders

I started working on this a bit. I'm putting up my current changes, it's in no way complete, or cleaned up, or anything. It makes all solid color border drawing pretty fast, except the cases where different sides have different widths. I'm working on improving more cases, but this code catches the majority of borders in our UI.

Comment 7

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Mask corners of some reftest due to AA differences.

We're not very consistent at how we AA single width rectangular border corners, this is fine and doesn't look bad an performs well. But we need to mask those corners in some reftests.

Comment 8

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Update masks to include extra pixels for new border rendering code.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Why do we need these mask pixels? Even though you're changing the behavior, shouldn't it remain consistent between test and reference?

Comment 10

[Bas Schouten (:bas.schouten)]

Attachment: Part 3: Update reftest that now fails on all platforms due to gradient rendering inconsistencies.

Comment 11

[Bas Schouten (:bas.schouten)]

Attachment: Part 4: Specialized border rendering code for common borders

This is the essence of this bug. The patch adds several specialized rendering paths that avoid using intermediate surfaces and use gradients for smooth corner transitions. This passes all tests with the patches acompanied in this patch. There's no clear differences on Talos (presumably because a bunch of the tests apparently don't count painting for automated Talos according to some bugs).

Locally running winopen.xul from stand alone Talos does show clear improvements (this is largely doing border rendering of our UI from what I can see).

D2D, without Patch: 80 ms
D2D, with Patch: 60 ms
D2D, No Borders at all: 50 ms
GDI, without Patch: 50 ms
GDI, with patch: 45ms
GDI, No Borders at all: 45ms

There's still some difference between D2D and GDI for opening a new window, but it's drastically reduced. The reason D2D still spends some considerable time drawing borders is that gradients are not cheap in D2D. I've got some ideas for improving this further on the backend level (a single, hard stop gradient could be done more cleverly than D2D's general gradient approach), but for now I think this patch will offer us the main improvement we're looking for. It also solves all dependent bugs.

Comment 12

[Bas Schouten (:bas.schouten)]

Attachment: Part 5: Mark reftests as no longer failing

These reftests used to seam slightly due to AA inconsistencies on the two different geometries blended with ADD. They no longer fail with the new gradient approach.

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 494112 [details] [diff] [review]
Part 3: Update reftest that now fails on all platforms due to gradient rendering inconsistencies.

I don't see why this should fail due to new border rendering. Can you upload the new images somewhere? Also, we should probably replace the reference rendering with something, perhaps per platform, that passes.

Comment 14

[Bas Schouten (:bas.schouten)]

Attachment: Part 3: Update reftest to mask nose edge

This patch masks the nose edge as per IRC discussion.

Comment 15

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 494466 [details] [diff] [review]
Part 3: Update reftest to mask nose edge

Looks good. Does this let us pass on all of the platforms?

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Bas, can you respond to comment #9?

Comment 17

[Bas Schouten (:bas.schouten)]

(In reply to comment #9)
> Why do we need these mask pixels? Even though you're changing the behavior,
> shouldn't it remain consistent between test and reference?

No, one uses an 'inset' border style, and the other draws the border manually. My new code has no specialized path for 'inset' and this will use the old code.

Comment 18

[Bas Schouten (:bas.schouten)]

http://hg.mozilla.org/mozilla-central/rev/554d37bb2aff
http://hg.mozilla.org/mozilla-central/rev/262d9ccb5da5
http://hg.mozilla.org/mozilla-central/rev/8edf6bda5fc2
http://hg.mozilla.org/mozilla-central/rev/9987d0724a10
http://hg.mozilla.org/mozilla-central/rev/ae06445eeff1