1328337 - NativeFontResource::CreateScaledFont implicitly truncates Float glyph size to uint32_t

Status: RESOLVED FIXED

(Core :: Printing: Output, defect, P1)

Description

[Lee Salzman [:lsalzman]]

Attachment: fix implicit conversion of glyph size from float to uint32_t in NativeFontResource::CreateScaledFont

Even though glyph size is passed through almost the entire printing pipeline as float, and even though RecordedScaledFontCreation records mGlyphSize as a Float, NativeFontResource::CreateScaledFont for some reason takes it as a uint32_t parameter, which causes it to implicitly truncate. This shouldn't happen.

Comment 1

[Bob Owen (:bobowen)]

Comment on attachment 8823366 [details] [diff] [review]
fix implicit conversion of glyph size from float to uint32_t in NativeFontResource::CreateScaledFont

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

I have no idea how I managed to do that, sorry.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aea479acaeb8
fix implicit conversion of glyph size from float to uint32_t in NativeFontResource::CreateScaledFont. r=bobowen

Comment 3

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/aea479acaeb8

Comment 4

[Bob Owen (:bobowen)]

Does this need uplifting?

Comment 5

[Lee Salzman [:lsalzman]]

(In reply to Bob Owen (:bobowen) from comment #4)
> Does this need uplifting?

I think we should be okay given that sandboxing is still only just riding the trains?

Comment 6

[Bob Owen (:bobowen)]

(In reply to Lee Salzman [:lsalzman] from comment #5)
> (In reply to Bob Owen (:bobowen) from comment #4)
> > Does this need uplifting?
> 
> I think we should be okay given that sandboxing is still only just riding
> the trains?

On Windows an initial sandbox is on release, so this code is being used now.

Comment 7

[Lee Salzman [:lsalzman]]

Comment on attachment 8823366 [details] [diff] [review]
fix implicit conversion of glyph size from float to uint32_t in NativeFontResource::CreateScaledFont

Approval Request Comment
[Feature/Bug causing the regression]: bug 1156742
[User impact if declined]: Fonts may print with slightly wrong font size when using E10s.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: aurora, beta
[Is the change risky?]: no
[Why is the change risky/not risky?]: Original behavior of truncating font sizes should never have been there in the first place, and this just gets rid of that.
[String changes made/needed]: None

Comment 8

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8823366 [details] [diff] [review]
fix implicit conversion of glyph size from float to uint32_t in NativeFontResource::CreateScaledFont

avoid implicit float to int conversion, aurora52+

The first RC for 51 is already built, so it might be too late for that...

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8823366 [details] [diff] [review]
fix implicit conversion of glyph size from float to uint32_t in NativeFontResource::CreateScaledFont

Too late for beta unless it's an emergency, but we can take this in 52.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dabf0d7affc2