Closed Bug 691581 Opened 9 years ago Closed 5 years ago

[WinXP] "ASSERTION: mFUnitsConvFactor not valid" with tiny font size

Categories

(Core :: Graphics, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: jruderman, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files, 1 obsolete file)

Attached file testcase
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.
Blocks: 670072
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.
Assignee: nobody → jfkthame
Still asserts on XP. Jonathan, is the patch in this bug still applicable?
Flags: needinfo?(jfkthame)
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.
Flags: needinfo?(jfkthame)
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.
(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 [1]. 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.

[1] 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/mozilla-central/rev/11486a275847
https://hg.mozilla.org/mozilla-central/rev/b0a8ab5e0856
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.