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)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
(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)
Attached patch patch v1 (obsolete) — Splinter Review
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+
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
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 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+
Ok, approval1.9+ WFM. :)
Whiteboard: [needs landing]
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
Attachment #300714 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: