Closed
Bug 415012
Opened 17 years ago
Closed 17 years ago
Fix deadSpaceGap computation and restore deadSpaceGap subtraction in PrintPreviewNavigate
Categories
(Core :: Print Preview, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file, 1 obsolete file)
1.70 KB,
patch
|
Details | Diff | Splinter Review |
I discovered this while testing the patch for bug 414075 (attachment 300540 [details] [diff] [review]).
I was checking whether we can re-enable subtraction of deadSpaceGap in the PrintPreview jump-to-page functionality, after applying that fix. (We can't -- we end up subtracting too much space right now)
I think it boils down to a difference that I can't explain between the value of presContext.mDeviceContext.mAppUnitsPerInch.
In nsSimplePageSequenceFrame::Reflow around line 250, these values are set:
aPresContext->mDeviceContext.mAppUnitsPerInch = 5760
gapInTwips = 360
deadSpaceGap = gapInTwips / 72 / 20 * mAppUnitsPerInch = 1440
... which means we put 1440 app units of dead space between each page.
In DocumentViewerImpl::PrintPreviewNavigate around line 3660, these values are set:
mPresContext->mRawPtr->mDeviceContext.mAppUnitsPerInch = 7740
deadSpaceGapTwips = 360
deadSpaceGap = deadSpaceGapTwips / 72 / 20 * mAppUnitsPerInch = 1935
... which means we're expecting 1935 app units of dead space above each page. (and we subtract that much space)
Due to this discrepancy, we end up with 1935 - 1440 = 495 app-units of the previous page visible in a jump-to-page operation.
At first I thought the discrepancy might be due to the print-preview scale, but it's not, AFAICT -- the value of that scale is:
mPrintEngine->mRawPtr->GetPrintPreviewScale() = 1.79166663
whereas the ratio of the mAppUnitsPerInch values is
1935/1440 = 1.34375
So, I'm not sure where the difference in mAppUnitsPerInch is coming from, but I think it's the source of the problem.
Comment 1•17 years ago
|
||
7740/5760 = 1935/1440 = 129/96 = 72*1.7916663/96. Printing is done to a 72dpi surface, and displays are normally around 96dpi. So your scaled printing DPI is 129dpi and your display is 96dpi.
Basically, one of them is using the printing context and one is using the display device context.
Assignee | ||
Comment 2•17 years ago
|
||
(In reply to comment #1)
> So your scaled printing DPI
> is 129dpi and your display is 96dpi.
>
> Basically, one of them is using the printing context and one is using the
> display device context.
Ah, interesting... Yeah, my laptop's display is 129 dpi, which has caused other issues in the past (bug 403511).
FWIW, here are two hacks that both fix the problem on my machine:
1. Manually forcing deadSpaceGap to be 1440 in PrintPreviewNavigate
2. Setting layout.css.dpi to 96 in about:config (which changes mAppUnitsPerInch in PrintPreviewNavigate to 5760, and hence makes deadSpaceGap come out to 1440)
Assignee | ||
Comment 3•17 years ago
|
||
This patch fixes the issue, and re-enables deadSpaceGap subtraction in PrintPreviewNavigate, by using the nsSequenceFrame's PresContext rather than the nsDocumentViewer's one for the TwipsToAppUnits conversion.
This change ensures that 'deadSpaceGap' will be in the same units in PrintPreviewNavigate as it was when the nsSimplePageSequenceFrame was laid out. These are the units we need it to be in, if we want to reliably scroll to a particular position on the seqFrame's view.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #300714 -
Flags: superreview?(roc)
Attachment #300714 -
Flags: review?(roc)
Attachment #300714 -
Flags: superreview?(roc)
Attachment #300714 -
Flags: superreview+
Attachment #300714 -
Flags: review?(roc)
Attachment #300714 -
Flags: review+
Assignee | ||
Updated•17 years ago
|
Summary: In Print Preview, deadSpaceGap & mAppUnitsPerInch have different values during reflow vs during jump-to-page → Fix deadSpaceGap computation and restore deadSpaceGap subtraction in PrintPreviewNavigate
Assignee | ||
Comment 4•17 years ago
|
||
Comment on attachment 300714 [details] [diff] [review]
patch v1
Requesting approval for inclusion in b3.
Justification:
- Minimal change, Print-preview-only, little (or no) risk
- Improves Print-Preview usability / prettiness. (restores inclusion of the dead-space above page in PrintPreview's NextPage/PrevPage/Jump-to-Page functionality)
Attachment #300714 -
Flags: approval1.9b3?
Comment 5•17 years ago
|
||
Comment on attachment 300714 [details] [diff] [review]
patch v1
This can wait until the next beta, IMO.
Attachment #300714 -
Flags: approval1.9b3?
Attachment #300714 -
Flags: approval1.9b3-
Attachment #300714 -
Flags: approval1.9+
Assignee | ||
Comment 6•17 years ago
|
||
Ok, approval1.9+ WFM. :)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [needs landing]
Assignee | ||
Comment 7•17 years ago
|
||
Here's the patch, as landed -- removed an obsolete comment and improved whitespace.
Checking in nsDocumentViewer.cpp;
/cvsroot/mozilla/layout/base/nsDocumentViewer.cpp,v <-- nsDocumentViewer.cpp
new revision: 1.570; previous revision: 1.569
done
Assignee | ||
Updated•17 years ago
|
Attachment #300714 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in
before you can comment on or make changes to this bug.
Description
•