Prefer performance over avoiding seaming

RESOLVED FIXED in mozilla16

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bas, Assigned: bas)

Tracking

(Blocks: 1 bug)

unspecified
mozilla16
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 632373 [details] [diff] [review]
Seam for more complex borders

Right now, for complex combinations of border characteristics we create temporary surfaces and use complex operators for each border corner. This is particular expensive on D2D but also isn't free on other drawing systems. This patch changes us to seam in those complex cases, there's precedent for this, IE9 actually almost always seams even on much simpler border cases. Chrome seams sometimes but not others, I don't know exactly when.
Attachment #632373 - Flags: review?(roc)

Updated

5 years ago
Blocks: 761388
(Assignee)

Updated

5 years ago
Duplicate of this bug: 761388

Updated

5 years ago
Blocks: 750871
No longer blocks: 761388

Updated

5 years ago
OS: Windows 7 → All
Hardware: x86_64 → All
(Assignee)

Comment 2

5 years ago
How do you feel about this roc?
Comment on attachment 632373 [details] [diff] [review]
Seam for more complex borders

Review of attachment 632373 [details] [diff] [review]:
-----------------------------------------------------------------

I feel fine.
Attachment #632373 - Flags: review?(roc) → review+
Comment on attachment 632373 [details] [diff] [review]
Seam for more complex borders

Might have been good to ask OS X how it felt about it, since as it turns out, it felt like it would have to do https://tbpl.mozilla.org/php/getParsedLog.php?id=12844359&tree=Mozilla-Inbound

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/68be2d36dd3f
Attachment #632373 - Flags: checkin-
(Assignee)

Comment 5

5 years ago
Invisible tiny single-subpixel difference. Any objection if I just add fuzz on OSX to this test?
I would just add border:none to the test and reference iframes.
(Assignee)

Comment 7

5 years ago
Created attachment 635571 [details] [diff] [review]
Part 0: Slightly change transform stresstest reftest.
Attachment #635571 - Flags: review?(roc)
Attachment #635571 - Flags: review?(roc) → review+
(Assignee)

Comment 8

5 years ago
Created attachment 635598 [details] [diff] [review]
Part 0: Mark reftest fuzzy on OS X

The previous patch had reftest issues on OS X 10.6 on try. I've reverted the test and now marked it fuzzy.
Attachment #635571 - Attachment is obsolete: true
Attachment #635598 - Flags: review?(roc)
Attachment #635598 - Flags: review?(roc) → review+
Should we consider making this change only in some cases rather than in all cases?  Perhaps based on either (1) the 'image-rendering' property or a new property like it (2) the width of the border or (3) a combination of the two?

(My sense is that some authors actually do care about getting correct rendering here, but the bulk of the performance problems are not for those authors.)
I'd rather not introduce any Web-facing API here. Eventually I think we can have correct rendering and performance.
A heuristic based on the width might be helpful, but I'm not sure what that should be.
(Assignee)

Comment 12

5 years ago
Right now this fallback codepath is also only called in the case of particularly complex borders that don't go through any of our fastpaths. (Unless we go through the 'don't stroke' codepath)

IE9 just seams everywhere. Should we run into web authors having trouble with the seams we could also look at introducing a fastpath for their particular usecase.
https://hg.mozilla.org/mozilla-central/rev/878a3e1f9591
https://hg.mozilla.org/mozilla-central/rev/9d076b46d1b7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Depends on: 768766
You need to log in before you can comment on or make changes to this bug.