Closed
Bug 1114336
Opened 9 years ago
Closed 9 years ago
Progressive tiling code has a typo with possibly bad consequences
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
People
(Reporter: kats, Assigned: nical)
Details
Attachments
(1 file)
1.58 KB,
patch
|
kats
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
bajaj
:
approval-mozilla-b2g34+
|
Details | Diff | Splinter Review |
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/TiledContentClient.cpp?rev=aa785fd0b16a#219 is missing the <= 2 check Saw this while reviewing the new contributor patch in bug 1113774. Don't have time at the moment to dig into the history or implications of this bug but i can do it tomorrow if nobody gets to it before me.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8540174 -
Flags: review?(bugmail.mozilla)
Reporter | ||
Comment 2•9 years ago
|
||
Comment on attachment 8540174 [details] [diff] [review] Fix the typo. Review of attachment 8540174 [details] [diff] [review]: ----------------------------------------------------------------- This might need uplifting, please check to see which branches this affects and uplift as needed.
Attachment #8540174 -
Flags: review?(bugmail.mozilla) → review+
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → nical.bugzilla
Assignee | ||
Comment 3•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0076eb0ed38c
Assignee | ||
Comment 4•9 years ago
|
||
The typo landed a year ago so pretty much all branches are affected. I'll let it bake on central for a day or two and see how things go. Not sure whether we want to uplift all the way to beta considering we haven't seen that it caused issues so far (or haven't made the connection).
Assignee | ||
Updated•9 years ago
|
status-b2g-v2.0:
--- → affected
status-b2g-v2.0M:
--- → affected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → affected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
status-firefox37:
--- → affected
Reporter | ||
Comment 5•9 years ago
|
||
It might be related to the things benwa is trying to fix in bug 1112332.
Comment on attachment 8540174 [details] [diff] [review] Fix the typo. Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: We can get into incorrect situations with low precision tiling, currently defaults on B2G and Mac. [Describe test coverage new/current, TBPL]: [Risks and why]: Low risk. [String/UUID change made/needed]:
Attachment #8540174 -
Flags: approval-mozilla-beta?
Attachment #8540174 -
Flags: approval-mozilla-b2g34?
Attachment #8540174 -
Flags: approval-mozilla-aurora?
[Blocking Requested - why for this release]: We heavily use tiling, including low precision on B2G, every time we scroll. This problem can lead to noticeable but hard to diagnose correctness problems.
blocking-b2g: --- → 2.1?
Updated•9 years ago
|
blocking-b2g: 2.1? → 2.1+
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0076eb0ed38c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Updated•9 years ago
|
Attachment #8540174 -
Flags: approval-mozilla-beta?
Attachment #8540174 -
Flags: approval-mozilla-beta+
Attachment #8540174 -
Flags: approval-mozilla-aurora?
Attachment #8540174 -
Flags: approval-mozilla-aurora+
Comment 9•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/f9b45b037664 https://hg.mozilla.org/releases/mozilla-beta/rev/ddb6efaa11ad
Comment 10•9 years ago
|
||
Is this bad enough to warrant 2.0 blocking status (and a b2g32 approval request)?
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #10) > Is this bad enough to warrant 2.0 blocking status (and a b2g32 approval > request)? I don't know how bad things need to be to warrant 2.0 blocking status, so I don't think I am the best person to answer this question. The fix is rather simple and low-risk so I would argue if favour of taking it to 2.0. forwarding the needinfo to Kats.
Flags: needinfo?(nical.bugzilla) → needinfo?(bugmail.mozilla)
Reporter | ||
Comment 12•9 years ago
|
||
I would say no. I don't know how many people are running 2.0 within Mozilla anymore and if this patch were to cause regressions it's unlikely it would be noticed before it shipped to users. Based on the testing so far we don't have any concrete evidence that this typo actually manifests as something bad so I would say to leave it out of 2.0.
Flags: needinfo?(bugmail.mozilla)
Updated•9 years ago
|
Attachment #8540174 -
Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
You need to log in
before you can comment on or make changes to this bug.
Description
•