ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

tdf#163194: Make width of comments in margin fixed on print / PDF export

Browse Source 
Previously, the width of the comments in margin on print / PDF export
was calculated using the same width factor, as when shown in the UI.
Before commit ac2720dcbe (tdf#73953 sw:
Allow resizing the comment section, 2024-01-11), there was no way to
change that, and the factor was hardcoded as 1.8 (and the print / PDF
export implementation in SwViewShell::PrintOrPDFExport relied on that).
The mentioned commit introduced a configuration DisplayWidthFactor,
that stored the user-defined width factor; and then in commit
cf9d8631ee (Related tdf#73953 Increase
default comment width, 2024-01-11), its default value was increased to
3.0, which immediately affected the width of the comments in print and
PDF output. The new default created comments that were too wide for
the page, even with default DPI.

There was another problem with existing implementation: the comment
width depended on DPI. It meant, that when a HiDPI monotor was used,
the comments printed / exported in margins were narrower, than when
it was done using a standard-DPI monitor. On HiDPI systems, a larger
width factor was necessary to see the "comment doesn't fit into the
page" problem.

Here, the old behavior is restored, where print and PDF output use a
fixed value of the comment width factor. Also, the output width now
doesn't depend on DPI.

Two other possibilities were considered:

1. Keep using user-defined width factor, as in UI, and calculate the
   correct comment position, as well as original page scaling factor,
   to fit them all on the output page;

2. Introduce a new configuration, dedicated for width of comments in
   margins of pages in print / PDF export (and, again, calculate the
   layout using that factor).

They both were rejected, because it doesn't make sense to reduce the
original page size too much. Currently, the original page is reduced
by factor of 0.75, to make room for the comments. Allowing more space
for comments could easily make the page reduced to one quarter of the
original size, or even smaller. And the text of comments will scale
down as well, making it unreadable with too wide comments area.

One problem that required solving was that dynamically changing the
width factor changed sizes of the comments, and that needed to be
restored (otherwise, exporting to PDF would change all UI sizes of
comments). For that, a good guess of the final comment text height
was needed; previously, it worked because comments were re-layouted
several times before they were finally shown, and that compensated
for the incorrect initial height used in SwPostItMgr::LayoutPostIts.
But for print / output, where there is only one layout for output,
and one layout for restoring the size, a new method was introduced
in SwAnnotationWin, GuessTextHeightForWidth.

This restores testTdf152575 to the original state before the commit
cf9d8631ee.

Change-Id: Ib0dd195d7594b51be86e489cc06cae00b26bf29b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/198457
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Jenkins
This commit is contained in:
Mike Kaganski
2026-01-31 19:10:08 +01:00
parent 384f2a0f46
commit fb52efb4ae
8 changed files with 229 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sw/inc/AnnotationWin.hxx
+1
View File
@@ -111,6 +111,7 @@ class SAL_DLLPUBLIC_RTTI SwAnnotationWin final : public InterimItemWindow
::sw::overlay::OverlayRanges* TextRange() { return mpTextRangeOverlay.get();}
tools::Long GetPostItTextHeight();
tools::Long GuessTextHeightForWidth(tools::Long nWidth) const;
void SwitchToPostIt(sal_uInt16 aDirection);
void SwitchToFieldPos();
sw/inc/PostItMgr.hxx
+2 -2
View File
@@ -99,7 +99,7 @@ class SAL_DLLPUBLIC_RTTI SwPostItMgr final : public SfxListener,
FieldShadowState mShadowState;
std::optional<OutlinerParaObject> mpAnswer;
OUString maAnswerText;
bool mbForceShow = false;
bool mbForceShowForPrintOrPdf = false;
// data structure to collect the <SwAnnotationWin> instances for certain <SwFrame> instances.
std::unique_ptr<sw::sidebarwindows::SwFrameSidebarWinContainer> mpFrameSidebarWinContainer;
@@ -255,7 +255,7 @@ class SAL_DLLPUBLIC_RTTI SwPostItMgr final : public SfxListener,
sw::sidebarwindows::SidebarPosition GetSidebarPos(const Point& rPointLogic);
void SetForceShow(bool bForce) { mbForceShow = bForce; }
void SetForceShowForPrintOrPdf(bool bForce) { mbForceShowForPrintOrPdf = bForce; }
// The commands that directly delete comments (as opposed to deletion of the text that the
// comments are anchored to) behave differently, depending on the document type, when the
sw/qa/extras/uiwriter/data/tdf163194-two-comments.fodt
+41
View File
@@ -0,0 +1,41 @@
<?xml version="1.0" encoding="UTF-8"?>
<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:meta="urn:oasis:names:tc:opendocument:xmlns:meta:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:svg="urn:oasis:names:tc:opendocument:xmlns:svg-compatible:1.0" office:version="1.4" office:mimetype="application/vnd.oasis.opendocument.text">
<office:font-face-decls>
<style:font-face style:name="Liberation Serif" svg:font-family="&apos;Liberation Serif&apos;" style:font-family-generic="roman" style:font-pitch="variable"/>
</office:font-face-decls>
<office:styles>
<style:default-style style:family="paragraph">
<style:paragraph-properties fo:orphans="2" fo:widows="2" style:text-autospace="ideograph-alpha" style:punctuation-wrap="hanging" style:line-break="strict" style:tab-stop-distance="12.51mm" style:writing-mode="page"/>
<style:text-properties style:font-name="Liberation Serif" fo:font-size="12pt" fo:language="en" fo:country="GB" style:letter-kerning="true"/>
</style:default-style>
<style:style style:name="Standard" style:family="paragraph" style:class="text"/>
<style:style style:name="Comment" style:family="paragraph" style:parent-style-name="Standard" style:class="extra">
<style:paragraph-properties fo:margin-left="1mm" fo:margin-right="1mm" fo:margin-top="1mm" fo:margin-bottom="0mm" style:contextual-spacing="false" fo:line-height="100%" fo:text-indent="0mm" style:auto-text-indent="false"/>
<style:text-properties fo:font-size="10pt"/>
</style:style>
</office:styles>
<office:automatic-styles>
<style:page-layout style:name="pm1">
<style:page-layout-properties fo:page-width="210mm" fo:page-height="297mm" style:print-orientation="portrait" fo:margin-top="20mm" fo:margin-bottom="20mm" fo:margin-left="20mm" fo:margin-right="20mm"/>
</style:page-layout>
</office:automatic-styles>
<office:master-styles>
<style:master-page style:name="Standard" style:page-layout-name="pm1"/>
</office:master-styles>
<office:body>
<office:text>
<text:p>He heard quiet steps behind him<office:annotation>
<dc:creator>Mike</dc:creator>
<dc:date>2026-01-31T17:49:06.393884900</dc:date>
<meta:creator-initials>M</meta:creator-initials>
<text:p text:style-name="Comment">A comment. One two three four five six seven eight nine ten eleven twelve thirteen fourteen fifteen sixteen seventeen eighteen nineteen twenty.</text:p>
</office:annotation>. That didn&apos;t bode well. Who could be following him this late at night and in this deadbeat part of town? And at this particular moment, just after he pulled off the big time and was making off with the greenbacks<office:annotation>
<dc:creator>Mike</dc:creator>
<dc:date>2026-01-31T17:51:06.483107200</dc:date>
<meta:creator-initials>M</meta:creator-initials>
<text:p text:style-name="Comment">Another comment. Twenty-one twenty-two twenty-three twenty-four twenty-five twenty-six twenty-seven twenty-eight twenty-nine thirty.</text:p>
</office:annotation>. Was there another crook who&apos;d had the same idea, and was now watching him and waiting for a chance to grab the fruit of his labour? Or did the steps behind him mean that one of many law officers in town was on to him and just waiting to pounce and snap those cuffs on his wrists? He nervously looked all around. Suddenly he saw the alley. Like lightning he darted off to the left and disappeared between the two warehouses almost falling over the bin lying in the middle of the pavement. He tried to nervously tap his way along in the inky darkness and suddenly stiffened: it was a dead-end, he would have to go back the way he had come. The steps got louder and louder, he saw the black outline of a figure coming around the corner. Is this the end of the line? he thought pressing himself back against the wall trying to make himself invisible in the dark, was all that planning and energy wasted? He was dripping with sweat now, cold and wet, he could smell the fear coming off his clothes. Suddenly next to him, with a barely noticeable squeak, a door swung quietly to and fro in the night&apos;s breeze. Could this be the haven he&apos;d prayed for? Slowly he slid toward the door, pressing himself more and more into the wall, into the dark, away from his enemy. Would this door save his hide?</text:p>
</office:text>
</office:body>
</office:document>
sw/qa/extras/uiwriter/uiwriter11.cxx
+136
View File
@@ -8,6 +8,9 @@
*/
#include <swmodeltestbase.hxx>
#include <officecfg/Office/Writer.hxx>
#include <vcl/pdf/PDFPageObjectType.hxx>
#include <vcl/scheduler.hxx>
#include <comphelper/propertyvalue.hxx>
@@ -470,6 +473,139 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest11, testTdf162120AutoRTLDefault)
CPPUNIT_ASSERT(getProperty<bool>(getRun(getParagraph(1), 1), u"WritingModeAutomatic"_ustr));
}
CPPUNIT_TEST_FIXTURE(SwUiWriterTest11, testTdf163194)
{
// Test (1) that exporting comments in margin to PDF produces expected layout of the comments,
// that have limited and reasonable width and position, inside the page bounds (and close to
// its right side), independent on the manually set comment sidebar width; and (2) that the
// export doesn't change the annotation sizes in the document.
// Annotation size depends on DisplayWidthFactor. Set it to a fixed value for testing.
comphelper::ScopeGuard
aReset([oldValue = officecfg::Office::Writer::Notes::DisplayWidthFactor::get()]() {
auto pChanges(comphelper::ConfigurationChanges::create());
officecfg::Office::Writer::Notes::DisplayWidthFactor::set(oldValue, pChanges);
pChanges->commit();
});
{
auto pChanges(comphelper::ConfigurationChanges::create());
officecfg::Office::Writer::Notes::DisplayWidthFactor::set(4.0, pChanges);
pChanges->commit();
}
createSwDoc("tdf163194-two-comments.fodt");
SwDocShell* pDocShell = getSwDocShell();
CPPUNIT_ASSERT(pDocShell);
SwView* pView = pDocShell->GetView();
CPPUNIT_ASSERT(pView);
SwPostItMgr* pPostItMgr = pView->GetPostItMgr();
CPPUNIT_ASSERT(pPostItMgr);
CPPUNIT_ASSERT_EQUAL(size_t(2), pPostItMgr->GetPostItFields().size());
std::vector<tools::Long> aOriginalHeights;
for (const auto& pAnnotationItem : pPostItMgr->GetPostItFields())
{
CPPUNIT_ASSERT(pAnnotationItem);
CPPUNIT_ASSERT(pAnnotationItem->mpPostIt);
const Size aSize = pAnnotationItem->mpPostIt->GetSizePixel();
// Pixel width depends on DisplayWidthFactor, seems to not depend on DPI
CPPUNIT_ASSERT_EQUAL(tools::Long(400), aSize.Width());
// Heights depend on width and current DPI scaling; just remember them for later comparison
aOriginalHeights.push_back(aSize.Height());
}
// Export to PDF with comments in margin
uno::Sequence aFilterData{ comphelper::makePropertyValue(u"ExportNotes"_ustr, false),
comphelper::makePropertyValue(u"ExportNotesInMargin"_ustr, true) };
uno::Sequence aDescriptor{ comphelper::makePropertyValue(u"FilterData"_ustr, aFilterData),
comphelper::makePropertyValue(u"URL"_ustr, maTempFile.GetURL()) };
dispatchCommand(mxComponent, u".uno:ExportToPDF"_ustr, aDescriptor);
if (auto pPdfDocument = parsePDFExport()) // This part will be skipped without PDFium
{
CPPUNIT_ASSERT_EQUAL(1, pPdfDocument->getPageCount());
auto pPage = pPdfDocument->openPage(0);
CPPUNIT_ASSERT(pPage);
// 595 pt corresponts to 210 mm (withh of A4)
CPPUNIT_ASSERT_DOUBLES_EQUAL(595, pPage->getWidth(), 1.0);
auto pTextPage = pPage->getTextPage();
CPPUNIT_ASSERT(pTextPage);
// There should be two filled path objects corresponding to the two annotations
int nPathCount = 0;
// Check that there are all the expected lines of the annotations, i.e. that the layout
// of the comments is stable and independent of the DPI.
const OUString aLines[] = {
// First annotation
u"A comment. One two three four "_ustr,
u"five six seven eight nine ten "_ustr,
u"eleven twelve thirteen fourteen "_ustr,
u"fifteen sixteen seventeen "_ustr,
u"eighteen nineteen twenty."_ustr,
// Second annotation
u"Another comment. Twenty-one "_ustr,
u"twenty-two twenty-three "_ustr,
u"twenty-four twenty-five twenty-"_ustr,
u"six twenty-seven twenty-eight "_ustr,
u"twenty-nine thirty."_ustr,
};
std::set<OUString> aFoundLines;
for (int i = 0; i < pPage->getObjectCount(); ++i)
{
auto pObject = pPage->getObject(i);
if (pObject->getType() == vcl::pdf::PDFPageObjectType::Path
&& pObject->getPathSegmentCount() == 5)
{
// This is the filled rectangle of an annotation. I hope that path segment count
// is stable and unique enough to identify it.
CPPUNIT_ASSERT(pObject->getFillColor() != COL_TRANSPARENT);
basegfx::B2DRectangle bounds = pObject->getBounds();
// The object must start at position 448, and have width 101 (so its right
// edge is ~at position 549, within the page's width of 595).
// Without the fix, this failed like "Expected: 101; Actual: 225", i.e. its
// right edge protruded beyond the right page limit (and generally depended
// on the UI width of the comment sidebar).
CPPUNIT_ASSERT_DOUBLES_EQUAL(101, bounds.getWidth(), 1.0);
CPPUNIT_ASSERT_DOUBLES_EQUAL(448, bounds.getMinX(), 1.0);
++nPathCount;
}
else if (pObject->getType() == vcl::pdf::PDFPageObjectType::Text)
{
// This is text; check if it matches expected annotation lines
OUString aText = pObject->getText(pTextPage);
if (std::ranges::find(aLines, aText) != std::end(aLines))
{
// Check that it fits into the expected horizontal range of the comment
basegfx::B2DRectangle bounds = pObject->getBounds();
CPPUNIT_ASSERT_LESS(101.0, bounds.getWidth());
CPPUNIT_ASSERT_DOUBLES_EQUAL(450, bounds.getMinX(), 1.0);
CPPUNIT_ASSERT(!aFoundLines.contains(aText)); // each line only once
aFoundLines.insert(aText);
}
}
}
CPPUNIT_ASSERT_EQUAL(2, nPathCount); // all annotation rectangles
CPPUNIT_ASSERT_EQUAL(std::size(aLines), aFoundLines.size()); // all annotation lines
}
// Test that export didn't change the annotation sizes
CPPUNIT_ASSERT_EQUAL(size_t(2), pPostItMgr->GetPostItFields().size());
for (size_t i = 0; i < 2; ++i)
{
const auto& pAnnotationItem = pPostItMgr->GetPostItFields()[i];
CPPUNIT_ASSERT(pAnnotationItem);
CPPUNIT_ASSERT(pAnnotationItem->mpPostIt);
const Size aSize = pAnnotationItem->mpPostIt->GetSizePixel();
CPPUNIT_ASSERT_EQUAL(tools::Long(400), aSize.Width());
CPPUNIT_ASSERT_EQUAL(aOriginalHeights[i], aSize.Height());
}
}
} // end of anonymous namespace
CPPUNIT_PLUGIN_IMPLEMENT();
sw/qa/extras/uiwriter/uiwriter8.cxx
+1 -1
View File
@@ -831,7 +831,7 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest8, testTdf152575)
std::unique_ptr<vcl::pdf::PDFiumPage> pPdfPage = pPdfDocument->openPage(/*nIndex=*/1);
CPPUNIT_ASSERT(pPdfPage);
// Without the fix for tdf#152575 this would be only 42 objects
CPPUNIT_ASSERT_EQUAL(50, pPdfPage->getObjectCount());
CPPUNIT_ASSERT_EQUAL(51, pPdfPage->getObjectCount());
}
CPPUNIT_TEST_FIXTURE(SwUiWriterTest8, testTdf140731)
sw/source/core/view/vprint.cxx
+8 -2
View File
@@ -520,12 +520,18 @@ bool SwViewShell::PrintOrPDFExport(
if (pPostItManager)
{
pPostItManager->SetForceShow(true);
pPostItManager->SetForceShowForPrintOrPdf(true);
pPostItManager->CalcRects();
pPostItManager->LayoutPostIts();
pPostItManager->DrawNotesForPage(pOutDev, nPage-1);
oOrigHeight.emplace(pStPage->getFrameArea().Height());
pPostItManager->SetForceShow(false);
pPostItManager->SetForceShowForPrintOrPdf(false);
if (pPostItManager->ShowNotes())
{
// In Print/Pdf mode, LayoutPostIts used a fixed sidebar width. Now restore the
// layout with the normal (user-defined) sidebar width.
pPostItManager->LayoutPostIts();
}
}
}
sw/source/uibase/docvw/AnnotationWin2.cxx
+14
View File
@@ -1197,6 +1197,20 @@ tools::Long SwAnnotationWin::GetPostItTextHeight()
return mpOutliner ? LogicToPixel(mpOutliner->CalcTextSize()).Height() : 0;
}
// Provides an estimation for the text height given a specific width of the annotation window.
// It doesn't calculate it accurately, e.g. it doesn't take scrollbars into account (see
// SwAnnotationWin::DoResize for more details of accurate calculation). Even though it avoids
// some calculations, it may still be quite expensive for large text.
tools::Long SwAnnotationWin::GuessTextHeightForWidth(tools::Long nWidth) const
{
if (!mpOutliner)
return 0;
comphelper::ScopeGuard resetPaperSize([this, curSize = mpOutliner->GetPaperSize()]()
{ mpOutliner->SetPaperSize(curSize); });
mpOutliner->SetPaperSize(PixelToLogic(Size(nWidth, SAL_MAX_INT32)));
return LogicToPixel(mpOutliner->CalcTextSize()).Height();
}
void SwAnnotationWin::SwitchToPostIt(sal_uInt16 aDirection)
{
SwAnnotationWin* pPostIt = mrMgr.GetNextPostIt(aDirection, this);
sw/source/uibase/docvw/PostItMgr.cxx
+26 -10
View File
@@ -982,6 +982,7 @@ void SwPostItMgr::LayoutPostIts()
// - place SwPostIts on their initial position
// - calculate necessary height for all PostIts together
bool bUpdate = false;
const tools::Long nSidebarWidth = GetSidebarWidth(true);
for (std::unique_ptr<SwPostItPageItem>& pPage : mPages)
{
// only layout if there are notes on this page
@@ -1020,7 +1021,7 @@ void SwPostItMgr::LayoutPostIts()
if (pPage->eSidebarPosition == sw::sidebarwindows::SidebarPosition::LEFT )
{
// x value for notes positioning
mlPageBorder = mpEditWin->LogicToPixel( Point( pPage->mPageRect.Left(), 0)).X() - GetSidebarWidth(true);// - GetSidebarBorderWidth(true);
mlPageBorder = mpEditWin->LogicToPixel(Point(pPage->mPageRect.Left(), 0)).X() - nSidebarWidth;// - GetSidebarBorderWidth(true);
//bending point
mlPageEnd =
mpWrtShell->getIDocumentSettingAccess().get(DocumentSettingId::BROWSE_MODE)
@@ -1040,13 +1041,18 @@ void SwPostItMgr::LayoutPostIts()
tools::Long Y = mpEditWin->LogicToPixel( Point(0,pItem->maLayoutInfo.mPosition.Bottom())).Y();
// Without taking new width into account, the text height will be wrong.
// GuessTextHeightForWidth is expensive, only use it when necessary.
tools::Long nTextHeight;
if (pPostIt->GetSizePixel().Width() == nSidebarWidth)
nTextHeight = pPostIt->GetPostItTextHeight();
else
nTextHeight = pPostIt->GuessTextHeightForWidth(nSidebarWidth);
tools::Long postItPixelTextHeight
= (comphelper::LibreOfficeKit::isActive()
? mpEditWin
->LogicToPixel(
Point(0, pPostIt->GetPostItTextHeight()))
.Y()
: pPostIt->GetPostItTextHeight());
? mpEditWin->LogicToPixel(Point(0, nTextHeight)).Y()
: nTextHeight);
aPostItHeight
= (postItPixelTextHeight < pPostIt->GetMinimumSizeWithoutMeta()
? pPostIt->GetMinimumSizeWithoutMeta()
@@ -1054,7 +1060,7 @@ void SwPostItMgr::LayoutPostIts()
+ pPostIt->GetMetaHeight();
pPostIt->SetPosSizePixelRect( mlPageBorder ,
Y - GetInitialAnchorDistance(),
GetSidebarWidth(true),
nSidebarWidth,
aPostItHeight,
mlPageEnd );
}
@@ -2419,7 +2425,7 @@ void SwPostItMgr::CorrectPositions()
bool SwPostItMgr::ShowNotes() const
{
// we only want to see notes if Options - Writer - View - Notes is ticked
return mbForceShow || mpWrtShell->GetViewOptions()->IsPostIts();
return mbForceShowForPrintOrPdf || mpWrtShell->GetViewOptions()->IsPostIts();
}
bool SwPostItMgr::HasNotes() const
@@ -2473,8 +2479,18 @@ tools::ULong SwPostItMgr::GetSidebarWidth(bool bPx) const
double fScaleX = double(mpWrtShell->GetOut()->GetMapMode().GetScaleX());
nZoom = fScaleX * 100;
}
tools::ULong aWidth = static_cast<tools::ULong>(
nZoom * officecfg::Office::Writer::Notes::DisplayWidthFactor::get());
double fDisplayWidthFactor;
if (mbForceShowForPrintOrPdf)
{
// 1.8 is the fixed factor that empirically matches (at 96 PPI) the 0.75 scale applied to
// the page in SwViewShell::PrintOrPDFExport for "print in margins" mode.
fDisplayWidthFactor = 1.8 * (Application::GetDefaultDevice()->GetDPIX() / 96.0);
}
else
{
fDisplayWidthFactor = officecfg::Office::Writer::Notes::DisplayWidthFactor::get();
}
tools::ULong aWidth = static_cast<tools::ULong>(nZoom * fDisplayWidthFactor);
if (bPx)
return aWidth;