Closed Bug 588271 Opened 14 years ago Closed 14 years ago

Improve border rendering code for running on GPUs


(Core :: Graphics, defect)

Not set



Tracking Status
blocking2.0 --- betaN+


(Reporter: jrmuizel, Assigned: bas.schouten)


(Blocks 1 open bug)



(5 files, 2 obsolete files)

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.
Blocks: 598202
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.
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).
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.
Blocks: 593812
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.
Attached patch First code on fast borders (obsolete) — Splinter Review
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.
Assignee: nobody → bas.schouten
blocking2.0: --- → betaN+
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.
Attachment #494109 - Flags: review?(roc)
Why do we need these mask pixels? Even though you're changing the behavior, shouldn't it remain consistent between test and reference?
Attachment #494112 - Attachment is patch: true
Attachment #494112 - Attachment mime type: application/octet-stream → text/plain
Blocks: 597540
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.
Attachment #494166 - Flags: review?(vladimir)
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.
Attachment #494193 - Flags: review?(vladimir)
Attachment #485613 - Attachment is obsolete: true
Attachment #494111 - Flags: review?(jmuizelaar) → review+
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.
Attachment #494112 - Flags: review?(jmuizelaar) → review-
This patch masks the nose edge as per IRC discussion.
Attachment #494112 - Attachment is obsolete: true
Attachment #494466 - Flags: review?(jmuizelaar)
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?
Attachment #494466 - Flags: review?(jmuizelaar) → review+
(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.
Blocks: 590407
Blocks: 591361
Depends on: 616510
Depends on: 616495
Depends on: 644104
Depends on: 679335
Depends on: 646053
Depends on: 773203
Depends on: 1303094
You need to log in before you can comment on or make changes to this bug.