Last Comment Bug 764117 - Prefer performance over avoiding seaming
: Prefer performance over avoiding seaming
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: mozilla16
Assigned To: Bas Schouten (:bas.schouten)
:
Mentors:
: 761388 (view as bug list)
Depends on: 768766
Blocks: 750871
  Show dependency treegraph
 
Reported: 2012-06-12 13:04 PDT by Bas Schouten (:bas.schouten)
Modified: 2012-06-26 21:14 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Seam for more complex borders (3.18 KB, patch)
2012-06-12 13:04 PDT, Bas Schouten (:bas.schouten)
roc: review+
philringnalda: checkin-
Details | Diff | Review
Part 0: Slightly change transform stresstest reftest. (1.75 KB, patch)
2012-06-21 18:47 PDT, Bas Schouten (:bas.schouten)
roc: review+
Details | Diff | Review
Part 0: Mark reftest fuzzy on OS X (1.14 KB, patch)
2012-06-21 21:46 PDT, Bas Schouten (:bas.schouten)
roc: review+
Details | Diff | Review

Description Bas Schouten (:bas.schouten) 2012-06-12 13:04:06 PDT
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.
Comment 1 Bas Schouten (:bas.schouten) 2012-06-13 02:29:08 PDT
*** Bug 761388 has been marked as a duplicate of this bug. ***
Comment 2 Bas Schouten (:bas.schouten) 2012-06-14 21:41:01 PDT
How do you feel about this roc?
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-06-18 21:50:29 PDT
Comment on attachment 632373 [details] [diff] [review]
Seam for more complex borders

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

I feel fine.
Comment 4 Phil Ringnalda (:philor) 2012-06-20 21:20:18 PDT
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
Comment 5 Bas Schouten (:bas.schouten) 2012-06-20 23:29:20 PDT
Invisible tiny single-subpixel difference. Any objection if I just add fuzz on OSX to this test?
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-06-21 04:01:20 PDT
I would just add border:none to the test and reference iframes.
Comment 7 Bas Schouten (:bas.schouten) 2012-06-21 18:47:40 PDT
Created attachment 635571 [details] [diff] [review]
Part 0: Slightly change transform stresstest reftest.
Comment 8 Bas Schouten (:bas.schouten) 2012-06-21 21:46:34 PDT
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.
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-06-21 23:00:02 PDT
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.)
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-06-22 04:22:51 PDT
I'd rather not introduce any Web-facing API here. Eventually I think we can have correct rendering and performance.
Comment 11 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-06-22 04:23:12 PDT
A heuristic based on the width might be helpful, but I'm not sure what that should be.
Comment 12 Bas Schouten (:bas.schouten) 2012-06-22 07:29:27 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.