Closed Bug 691581 Opened 9 years ago Closed 5 years ago
XP] "ASSERTION: m FUnits Conv Factor not valid" with tiny font size
On WinXP only (not Windows 7), tiny font sizes trigger: ###!!! ASSERTION: mFUnitsConvFactor not valid: 'mFUnitsConvFactor > 0.0f', file e:\builds\moz2_slave\try-w32-dbg\build\obj-firefox\dist\include\gfxFont.h, line 998
Hmm, guessing this'll be a result of bug 670072, which made us round fractional font sizes to whole pixels for GDI, which makes very small sizes go to zero.
Possible patch, ensure that the GDI font resource is created with a non-zero size (even if we're rounding our font size to zero) so that we don't hit degenerate cases when trying to get metrics. Does this resolve the problem (without regressing bug 670072)? It seems OK in my testing on Win7 (using GDI fonts), but I don't have an XP machine currently set up for easy testing.
Still asserts on XP. Jonathan, is the patch in this bug still applicable?
Probably; but AFAICS from comment 2, I never tested it under XP, and was hoping someone else would. The other thing is, I'm not sure it's really the right place to be fixing this. Could you post a stack for where we hit the assertion? I'm wondering whether we should be short-circuiting some code path if mAdjustedSize is zero, and thereby avoid this altogether.
Never mind, I found your try run and looked at where it's asserting. I think there's a more correct fix we can try; I'll post a patch shortly.
Attachment #564516 - Attachment is obsolete: true
Comment on attachment 8681691 [details] [diff] [review] Don't let a zero-sized font result in assertions from FUnitsToDevUnitsFactor() Review of attachment 8681691 [details] [diff] [review]: ----------------------------------------------------------------- Hmm, seems a bit dangerous to have a conversion factor that can go to zero but I guess we've commented where it's defined clearly.
Attachment #8681691 - Flags: review?(jdaggett) → review+
Also, please include a crashtest for this.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e7658cff5f572347f1d4c3eea412e71280e8d47 Bug 691581 - Don't let a zero-sized font result in assertions from FUnitsToDevUnitsFactor(). r=jdaggett https://hg.mozilla.org/integration/mozilla-inbound/rev/5cf33dcba0d02886e7f2c45e1bd7392a5f05d0d4 Bug 691581 - Add crashtest.
https://hg.mozilla.org/integration/mozilla-inbound/rev/214de8010262a5a0f4e7631ebc572bcfe0afdf7c Backed out changeset 5cf33dcba0d0 (bug 691581) for upcoming bustage https://hg.mozilla.org/integration/mozilla-inbound/rev/aee53bd3a239fbd917ae4726d341d9e0da5c520d Backed out changeset 6e7658cff5f5 (bug 691581) for upcoming bustage
(In reply to Jonathan Kew (:jfkthame) from comment #12) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=14a07aed4624 Note that WinXP jobs don't run by default on Try anymore . I've forced them to run on this push with mozci. Note that the crashtest was failing in comment 6 as well when they were forced to run.  try: -b d -p win32 -u crashtest[Windows XP] -t none
Ohhhhhh! Thanks. Wish I'd known that sooner.... I assumed they were just taking "forever" get scheduled for some infra reason.
https://hg.mozilla.org/integration/mozilla-inbound/rev/11486a275847896bf97beff11d6c07ffea3611e9 Bug 691581 - Don't let a zero-sized font result in assertions from FUnitsToDevUnitsFactor(). r=jdaggett https://hg.mozilla.org/integration/mozilla-inbound/rev/b0a8ab5e08568fc92f8f01db5c5cefaa663ed30f Bug 691581 - Add crashtest.
You need to log in before you can comment on or make changes to this bug.