Font size rounding bug

VERIFIED FIXED in M4

Status

P1
normal
VERIFIED FIXED
21 years ago
20 years ago

People

(Reporter: harishd, Assigned: harishd)

Tracking

other
x86
Windows NT

Details

(Assignee)

Description

21 years ago
Peter Linss wrote:

In working to make NGLayout's font handling match that of Nav's, I've uncovered
a rounding error in the Win libfont code:

modules/libfont/producers/win/src/winfp.cpp: line 875

// for passing to Windows, convert pointsize to pixel size and negative.
 if( pointSize > 0 )
  pixelSize = (int) - pointSize * pPrmFont->YPixelPerInch / 72.0;

pointSize is the input size (a double), the rounding error is introduced in the
conversion to pixel size.
The problem is a classic lack of parentheses, the int conversion is happening
before the divide by 72.0.
To do what was meant, the line should read:
  pixelSize = (int) - ((pointSize * pPrmFont->YPixelPerInch) / 72.0);
However, this is also wrong. The proper (ie: Windows) way is to round to the
rearest integer, not the smallest. According to the
Windows docs the code should read:
  pixelSize = - MulDiv((int)pointSize, pPrmFont->YPixelPerInch, 72);
That would give the correct rounding.

This bug results in Nav being unable to render certain font sizes, ie: 8 point
text comes out significantly smaller (more like 7
point).

For more information about the way it looks checkout the above URL.

Comment 1

21 years ago
dp, since this may require a code review to verify the fix, who would be the
appropriate engineer to look at the code.

Comment 2

21 years ago
assigning Fenella as QA Assigned to

Comment 3

21 years ago
tell me if this is a regression from 4.05...
This isn't a regression. We are still evaluating whether we need to fix it ASAP.
Data is pretty convoluted betn. 3.0,4.0,IE4.0,IE5.0 and word.

We should revisit this for 4.5

Comment 5

21 years ago
Putting on b2 radar.  Changing from P0 to P1.
This is not a regression nor a crash, nor data loss, nor an escalation,
therefore this bug doesn't meet the 4.5 criteria
(http://webgroup/novabugs.html).  Marking TFV 5.0.

Comment 7

20 years ago
Setting all current Open/Normal to M4.

Updated

20 years ago
Component: html display → FontLib

Updated

20 years ago
Status: NEW → RESOLVED
Last Resolved: 20 years ago
Resolution: --- → FIXED

Updated

20 years ago
QA Contact: 3849

Comment 8

20 years ago
Old bug.  beppe, couldyou assign QA to Verify this one please.

Comment 9

20 years ago
asking dp who is the best person to review this within fontlib.

Comment 10

20 years ago
I talked with Harish about how we could verify this bug and there really isn't
any way that we can do this visually. This would really need to be done through
assessing what the code pushes out. Maybe Peter Linss has a suggestion on how to
 verify this.

Comment 11

20 years ago
I talked with Harish about how we could verify this bug and there really isn't
any way that we can do this visually. This would really need to be done through
assessing what the code pushes out. Maybe Peter Linss has a suggestion on how to
 verify this.

Updated

20 years ago
Status: RESOLVED → VERIFIED

Comment 12

20 years ago
per Peters assessment, marking this as verified
You need to log in before you can comment on or make changes to this bug.