Closed
Bug 792314
Opened 12 years ago
Closed 12 years ago
Make print preview support display lists properly
Categories
(Core :: Print Preview, defect)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: mattwoodrow, Assigned: mattwoodrow)
References
Details
Attachments
(3 files, 4 obsolete files)
7.22 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
4.94 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
8.12 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
For DLBI to work correctly, we need the printing code (or reftests at least) to be building full display lists instead of recursing into nsLayoutUtils::PaintFrame. Looks like this patch works correctly when printing a page to pdf on Mac (with a page zoom forced via pref). Does anyone know how to test with a non-1.0 page sequence resolution? It doesn't appear to be used at all when printing on mac, and Mac doesn't have built-in print preview. Think there's still a few issues with reftests currently, working on it.
Attachment #662421 -
Flags: feedback?(roc)
Attachment #662421 -
Flags: feedback?(roc) → feedback+
Comment 1•12 years ago
|
||
For testing print preview on OSX there is the addon https://addons.mozilla.org/en-US/firefox/addon/print-print-preview-update/?src=ap . It's not the exact same thing, but seems to use most the print preview code.
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #662421 -
Attachment is obsolete: true
Attachment #662739 -
Flags: review?(roc)
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #662740 -
Flags: review?(roc)
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #662741 -
Flags: review?(roc)
Comment on attachment 662739 [details] [diff] [review] Make nsDisplayTransform support untransformed frames Review of attachment 662739 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsDisplayList.h @@ +2505,5 @@ > */ > class nsDisplayTransform: public nsDisplayItem > { > public: > + typedef gfx3DMatrix (* ComputeTransformFunction)(nsIFrame* aFrame, float aAppUnitsPerPixel); You need to document how the matrix returned by this function is interpreted.
Attachment #662739 -
Flags: review?(roc) → review+
Comment on attachment 662740 [details] [diff] [review] Build full display lists for nsPageFrame Review of attachment 662740 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsPageFrame.cpp @@ +405,4 @@ > NS_ENSURE_SUCCESS(rv, rv); > > + if (PresContext()->GetPageScale() != 1.0) { > + rv = content.AppendNewToTop(new (aBuilder) nsDisplayTransform(aBuilder, this, &content, ::ComputePageTransform)); Simpler to just always build this. No point in having multiple code paths to cause confusion later.
Attachment #662740 -
Flags: review?(roc) → review+
Comment on attachment 662741 [details] [diff] [review] Build full display lists for nsSimplePageSequence Review of attachment 662741 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsSimplePageSequence.cpp @@ +807,3 @@ > { > + float scale = aFrame->PresContext()->GetPrintPreviewScale(); > + return gfx3DMatrix::ScalingMatrix(scale, scale, 1); Why don't we need to change the basis here? Are we assuming that the nsSimplePageSequenceFrame is always at 0,0? That might not be a good assumption. @@ +825,3 @@ > > + if (PresContext()->GetPrintPreviewScale() != 1.0) { > + rv = content.AppendNewToTop(new (aBuilder) nsDisplayTransform(aBuilder, this, &content, ::ComputePageSequenceTransform)); Make this unconditional too.
Assignee | ||
Comment 8•12 years ago
|
||
Attachment #662740 -
Attachment is obsolete: true
Attachment #662761 -
Flags: review?(roc)
Assignee | ||
Comment 9•12 years ago
|
||
Test change forces active layers above it, just like the file it's being compared to, which has an active <canvas> object.
Attachment #662741 -
Attachment is obsolete: true
Attachment #662741 -
Flags: review?(roc)
Attachment #662762 -
Flags: review?(roc)
Assignee | ||
Comment 10•12 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #7) > Comment on attachment 662741 [details] [diff] [review] > Build full display lists for nsSimplePageSequence > > Review of attachment 662741 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/generic/nsSimplePageSequence.cpp > @@ +807,3 @@ > > { > > + float scale = aFrame->PresContext()->GetPrintPreviewScale(); > > + return gfx3DMatrix::ScalingMatrix(scale, scale, 1); > > Why don't we need to change the basis here? Are we assuming that the > nsSimplePageSequenceFrame is always at 0,0? That might not be a good > assumption. This was because nsPageFrame was transforming it's child, not itself and needed to adjust for that. But i've changed that now so the transform is created for the child directly. We still change the basis in both cases inside nsDisplayTransform::GetTransform.
Comment on attachment 662761 [details] [diff] [review] Build full display lists for nsPageFrame v2 Review of attachment 662761 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsPageFrame.cpp @@ +366,5 @@ > } > > +static gfx3DMatrix ComputePageTransform(nsIFrame* aFrame, float aAppUnitsPerPixel) > +{ > + float scale = aFrame->GetParent()->PresContext()->GetPageScale(); This GetParent() is superfluous.
Attachment #662761 -
Flags: review?(roc) → review+
Attachment #662762 -
Flags: review?(roc) → review+
Assignee | ||
Comment 12•12 years ago
|
||
Added back clipping code that I removed. This fixes a few reftest failures on tinderbox and exposed a regression from bug 788044.
Attachment #662761 -
Attachment is obsolete: true
Attachment #662785 -
Flags: review?(roc)
Attachment #662785 -
Flags: review?(roc) → review+
Assignee | ||
Comment 13•12 years ago
|
||
Landed the first 2 patches, the nsPageSequence one is still causing android fuzz failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/724f2a728a6f https://hg.mozilla.org/integration/mozilla-inbound/rev/f8e84de2291e Try push: https://tbpl.mozilla.org/?tree=Try&rev=030841d5fa13
Whiteboard: [leave open]
Comment 14•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/724f2a728a6f https://hg.mozilla.org/mozilla-central/rev/f8e84de2291e
Flags: in-testsuite?
Assignee | ||
Comment 15•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6e7a1b07944
Assignee: nobody → matt.woodrow
Whiteboard: [leave open]
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e6e7a1b07944
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in
before you can comment on or make changes to this bug.
Description
•