Closed
Bug 1274726
Opened 8 years ago
Closed 8 years ago
Dragged tab's top edge has horizontal line rendering artifact during dragging when using a Firefox theme on OS X
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox46 | --- | unaffected |
firefox47 | --- | unaffected |
firefox48 | + | fixed |
firefox49 | + | fixed |
People
(Reporter: cpeterson, Assigned: mchang)
References
Details
(Keywords: regression)
Attachments
(3 files)
(deleted),
patch
|
jrmuizel
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jrmuizel
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
[Tracking Requested - why for this release]:
Mason, I believe this bug is a regression from bug 1266933.
STR:
1. Install a light-colored Firefox theme, such as https://addons.mozilla.org/en-US/firefox/addon/soft-summer-evening/
2. Open a couple tabs.
3. Drag one of the tabs left and right
RESULT:
The dragged tab's top edge shifts down a pixel, making the tab look broken. Even before Skia bug 1266933 landed, a dragged tab's top edge looked blurred, but now it's much worse.
The bug doesn't seem to be reproducible with the default Firefox theme. I don't know whether that is because the default gray colors make seeing the problem difficult (even when zooming the screen) or installing any theme causes a problem. I can reproduce this problem with both light and dark themes, but the problem is easier to see with a light theme.
Flags: needinfo?(mchang)
Comment 1•8 years ago
|
||
Tracking this visual regression for 49. Chris - is this supposed to be nominated for tracking-firefox-esr38 as well?
Flags: needinfo?(cpeterson)
Reporter | ||
Comment 2•8 years ago
|
||
[Tracking Requested - why for this release]:
(In reply to Marcia Knous [:marcia - use ni] from comment #1)
> Tracking this visual regression for 49. Chris - is this supposed to be
> nominated for tracking-firefox-esr38 as well?
No. Thanks for catching that! I meant to nominate for tracking 48 and 49.
Assignee | ||
Comment 3•8 years ago
|
||
Is it correct to say that this only happens when actually dragging the tab? When I lock the tab in place, the regression goes away.
Flags: needinfo?(mchang) → needinfo?(cpeterson)
Reporter | ||
Comment 4•8 years ago
|
||
(In reply to Mason Chang [:mchang] from comment #3)
> Is it correct to say that this only happens when actually dragging the tab?
> When I lock the tab in place, the regression goes away.
Yes. This problem only happens while dragging the tab.
Flags: needinfo?(cpeterson)
Summary: Dragged tab's top edge has horizontal line rendering artifact when using a Firefox theme on OS X → Dragged tab's top edge has horizontal line rendering artifact during dragging when using a Firefox theme on OS X
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mchang
Assignee | ||
Comment 6•8 years ago
|
||
When we're prescaling an image to repeat it, we first prescale the image then repeat the scaled image. During the prescaling transformation, we currently also extend the repeat mode [1]. Since we're only scaling the image, we can clamp the image during scaling, and only pass the repeat mode when we're actually repeating.
[1] https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxUtils.cpp?from=gfxUtils.cpp#606
Attachment #8755615 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 7•8 years ago
|
||
Try looks good - https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b1bbe4c32c4
Comment 8•8 years ago
|
||
Comment on attachment 8755615 [details] [diff] [review]
repeat.patch
Review of attachment 8755615 [details] [diff] [review]:
-----------------------------------------------------------------
Can you add a test for this?
Attachment #8755615 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 9•8 years ago
|
||
Specifically tests the prescale and repeat drawable on OS X only since that's the only place we do it. I'd be interested to see if we still have to do it anymore since Skia scales 3x faster than CG did.
Successful try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=097f514bc376
Attachment #8758940 -
Flags: review?(jmuizelaar)
Updated•8 years ago
|
Attachment #8758940 -
Flags: review?(jmuizelaar) → review+
Comment 10•8 years ago
|
||
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b4b4972c8a5
Clamp scaled image before repeating during prescale and repeat on OS X. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fe8a8bde7f0
Part 2: Reftest for prescale and repeat drawable. r=jrmuizel
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2b4b4972c8a5
https://hg.mozilla.org/mozilla-central/rev/0fe8a8bde7f0
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8755615 [details] [diff] [review]
repeat.patch
Approval Request Comment
[Feature/regressing bug #]: Skia content on OS X
[User impact if declined]: Some graphics artifacts
[Describe test coverage new/current, TreeHerder]: Manual, attached reftest.
[Risks and why]: Low, we are forcing Skia to copy CoreGraphics behavior.
[String/UUID change made/needed]: None
Attachment #8755615 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 13•8 years ago
|
||
Attachment #8758940 -
Flags: approval-mozilla-beta?
Comment 14•8 years ago
|
||
Comment on attachment 8755615 [details] [diff] [review]
repeat.patch
This fixes regression. Take it in beta.
Attachment #8755615 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 15•8 years ago
|
||
Comment on attachment 8758940 [details] [diff] [review]
Reftest
This fixes regression. Take it in beta.
Attachment #8758940 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 16•8 years ago
|
||
has problems uplifting to beta:
Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r 2b4b4972c8a5
grafting 348604:2b4b4972c8a5 "Bug 1274726. Clamp scaled image before repeating during prescale and repeat on OS X. r=jrmuizel"
merging gfx/thebes/gfxUtils.cpp
warning: conflicts while merging gfx/thebes/gfxUtils.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(mchang)
Assignee | ||
Comment 17•8 years ago
|
||
Includes part 1 and part 2. I didn't have merge conflicts. From comment 16, it looks like you're applying it to mozilla-central?
Flags: needinfo?(mchang) → needinfo?(cbook)
Comment 18•8 years ago
|
||
bugherder uplift |
Comment 19•8 years ago
|
||
(In reply to Mason Chang [:mchang] from comment #17)
> Created attachment 8763930 [details] [diff] [review]
> Rolled patch for beta
>
> Includes part 1 and part 2. I didn't have merge conflicts.
thanks this worked great !
From comment 16,
> it looks like you're applying it to mozilla-central?
oh not really, its a so called unified repo, that way we take whats landed on (in this case mozilla-central) and graft it to mozilla-beta, sorry for the confusion
Flags: needinfo?(cbook)
Updated•8 years ago
|
Version: unspecified → 48 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•