From c5592ec7db0dfbdcdf488a4de42acdf34bfda164 Mon Sep 17 00:00:00 2001 From: Mike Kaganski Date: Tue, 29 Sep 2026 11:06:18 +0500 Subject: [PATCH] tdf#163240 sw: build a link's rectangles from its line pieces Exporting a hyperlink laid out down the page, over more than one line, produced link annotations a PDF/UA validator rejects: on a vertical page whose link wraps, two of the three annotations carried no structure element, and veraPDF reported both under ISO 14289-1 7.18.5: A Link annotation is an Artifact or is nested within null tag (standard type = null) instead of Link Export a document whose hyperlink ends in a ruby and runs over several lines down the page to PDF with tagged PDF enabled, then run veraPDF over the result in PDF/UA-1 mode to see it. An annotation is tied to a structure element by the text painted inside it, so one holding no text of any line is tied to nothing. That is what the region CalcFrameRects returns can hand out: it covers the selection in whatever rectangles fall out of inverting the pieces it was built from, and one of those can span several lines while another holds a ruby and none of the text beside it. Let CalcFrameRects return those pieces as well, and the holes it cuts afterwards, and take the shape from the pieces and the area from the region: the pieces follow the lines and, once the overlapping ones are joined, no longer overlap each other, so every part of the region falls in exactly one of them. What tdf#157816 requires still decides each part, and the parts a piece has left are joined unless the join would reach over a hole - a fly, a drop cap - that the selection excludes on purpose. Every annotation now holds text of the lines it covers, so every one of them is tagged, and a ruby sits inside the rectangle of the line that carries it. The narrowing to a portion's band, the growing over a ruby portion and the joining of tiling rectangles that Ide896d00c7619b772802762e85abfb6e2cf7ef0a ("tdf#163240 sw: let a link's rectangle cover the ruby it ends in") added are all gone. On the horizontal document that change was measured against, the exported rectangles are the same to the last digit. Signed-off-by: Mike Kaganski Change-Id: I9faf2bc10af491576cc2f4ba5f3e3433eb03d16c Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/13767 Reviewed-by: Miklos Vajna Tested-by: Jenkins CPCI (cherry picked from commit e77b1d25d3601315f80169e4e811d74ef2ff0374) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/211704 Reviewed-by: Xisco Fauli Tested-by: Jenkins --- sw/inc/swregion.hxx | 6 + sw/qa/core/text/EnhancedPDFExportHelper.cxx | 25 +++ .../link-ends-in-ruby-vertical-wrapped.fodt | 34 +++ sw/source/core/bastyp/swregion.cxx | 2 + sw/source/core/inc/rootfrm.hxx | 7 +- sw/source/core/layout/trvlfrm.cxx | 9 +- .../core/text/EnhancedPDFExportHelper.cxx | 196 +++++++----------- 7 files changed, 161 insertions(+), 118 deletions(-) create mode 100644 sw/qa/core/text/data/link-ends-in-ruby-vertical-wrapped.fodt diff --git a/sw/inc/swregion.hxx b/sw/inc/swregion.hxx index 338a30649c7a..1a951ab1658b 100644 --- a/sw/inc/swregion.hxx +++ b/sw/inc/swregion.hxx @@ -35,6 +35,7 @@ typedef std::vector SwRects; class SwRegionRects : public SwRects { SwRect m_aOrigin; // Origin area, limits the total area e.g. for Invert() + SwRects* m_pSubtracted = nullptr; inline void InsertRect( const SwRect &rRect, const sal_uInt16 nPos, bool &rDel); @@ -45,6 +46,11 @@ public: // most likely want to call ChangeOrigin() afterwards. explicit SwRegionRects( sal_uInt16 nInit = 20 ); + // Append every rectangle punched from here on, for a caller that wants the pieces a + // selection was built from and not only the region Invert() leaves. Invert() punches + // into a region of its own, so its own work is not recorded. + void RecordSubtracted(SwRects* pRects) { m_pSubtracted = pRects; } + // For punching from aOrigin. void operator-=( const SwRect& rRect ); void operator+=( const SwRect& rRect ); diff --git a/sw/qa/core/text/EnhancedPDFExportHelper.cxx b/sw/qa/core/text/EnhancedPDFExportHelper.cxx index f77e3267bb29..714d06a5dcd9 100644 --- a/sw/qa/core/text/EnhancedPDFExportHelper.cxx +++ b/sw/qa/core/text/EnhancedPDFExportHelper.cxx @@ -540,6 +540,31 @@ CPPUNIT_TEST_FIXTURE(Test, testLinkEndsInRubyVertical) CPPUNIT_ASSERT_DOUBLES_EQUAL(20.7, aRects[0].getWidth(), 0.5); } +CPPUNIT_TEST_FIXTURE(Test, testLinkEndsInRubyVerticalWrapped) +{ + createSwDoc("link-ends-in-ruby-vertical-wrapped.fodt"); + + uno::Sequence aFilterData{ comphelper::makePropertyValue(u"UseTaggedPDF"_ustr, true) }; + save(TestFilter::PDF_WRITER, + { comphelper::makePropertyValue(u"FilterData"_ustr, aFilterData) }); + + vcl::filter::PDFDocument aDocument; + CPPUNIT_ASSERT(aDocument.Read(*maTempFile.GetStream(StreamMode::READ))); + + // an annotation for each of the three lines, all tagged, which the collector asserts + std::vector aRects(lcl_CollectTaggedLinkRects(aDocument)); + CPPUNIT_ASSERT_EQUAL(size_t(3), aRects.size()); + // the text runs down the page from the right, so the widest x is the first line + std::ranges::sort(aRects, + [](const basegfx::B2DRectangle& rLeft, const basegfx::B2DRectangle& rRight) { + return rLeft.getMinX() > rRight.getMinX(); + }); + // only the third line carries the ruby, and only it is as wide as a ruby makes a line + CPPUNIT_ASSERT_DOUBLES_EQUAL(13.8, aRects[0].getWidth(), 0.5); + CPPUNIT_ASSERT_DOUBLES_EQUAL(13.8, aRects[1].getWidth(), 0.5); + CPPUNIT_ASSERT_DOUBLES_EQUAL(20.7, aRects[2].getWidth(), 0.5); +} + CPPUNIT_TEST_FIXTURE(Test, testLinkPartRubyOneLine) { createSwDoc("link-part-ruby-one-line.fodt"); diff --git a/sw/qa/core/text/data/link-ends-in-ruby-vertical-wrapped.fodt b/sw/qa/core/text/data/link-ends-in-ruby-vertical-wrapped.fodt new file mode 100644 index 000000000000..a8763d79c69e --- /dev/null +++ b/sw/qa/core/text/data/link-ends-in-ruby-vertical-wrapped.fodt @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + A hyperlink long enough to wrap, ending in a phonetic annotation whose band is taller than the rest of the line A hyperlink long enough to wrap, ending in a phonetic annotation whose band is taller than the rest of the line A hyperlink long enough to wrap, ending in a phonetic annotation whose band is taller than the rest of the line A hyperlink long enough to wrap, ending in a phonetic annotation whose band is taller than the rest of the line baseruby + + + diff --git a/sw/source/core/bastyp/swregion.cxx b/sw/source/core/bastyp/swregion.cxx index 97e271811ab8..8872c2e85203 100644 --- a/sw/source/core/bastyp/swregion.cxx +++ b/sw/source/core/bastyp/swregion.cxx @@ -65,6 +65,8 @@ void SwRegionRects::operator+=( const SwRect &rRect ) */ void SwRegionRects::operator-=( const SwRect &rRect ) { + if (m_pSubtracted) + m_pSubtracted->push_back(rRect); sal_uInt16 nMax = size(); for ( sal_uInt16 i = 0; i < nMax; ++i ) { diff --git a/sw/source/core/inc/rootfrm.hxx b/sw/source/core/inc/rootfrm.hxx index 7813a79686a1..4b5c2029e6e6 100644 --- a/sw/source/core/inc/rootfrm.hxx +++ b/sw/source/core/inc/rootfrm.hxx @@ -342,7 +342,12 @@ public: bool IsBetweenPages(const Point& rPt) const; enum class RectsMode { Default, NoAnchoredFlys }; - void CalcFrameRects(SwShellCursor const&, SwRects &, RectsMode eMode = RectsMode::Default); + /// pPieces, when given, also receives the rectangles the selection was built from, + /// which follow the lines while the region left in the second argument need not, and + /// pHoles the ones cut out of it afterwards - a fly, a drop cap. The two account for + /// the region only in NoAnchoredFlys mode, where nothing is added back to it. + void CalcFrameRects(SwShellCursor const&, SwRects&, RectsMode eMode = RectsMode::Default, + SwRects* pPieces = nullptr, SwRects* pHoles = nullptr); /** * Calculates the cells included from the current selection diff --git a/sw/source/core/layout/trvlfrm.cxx b/sw/source/core/layout/trvlfrm.cxx index 85ce7e34b0e4..6cbb00a305f1 100644 --- a/sw/source/core/layout/trvlfrm.cxx +++ b/sw/source/core/layout/trvlfrm.cxx @@ -2113,7 +2113,8 @@ static void Add( SwRegionRects& rRegion, const SwRect& rRect ) * rectangles are available for highlighting. * In the end the Flys are cut out of the region. */ -void SwRootFrame::CalcFrameRects(SwShellCursor const& rCursor, SwRects & rRects, RectsMode const eMode) +void SwRootFrame::CalcFrameRects(SwShellCursor const& rCursor, SwRects& rRects, + RectsMode const eMode, SwRects* pPieces, SwRects* pHoles) { auto [pStartPos, pEndPos] = rCursor.StartEnd(); // SwPosition* @@ -2126,6 +2127,11 @@ void SwRootFrame::CalcFrameRects(SwShellCursor const& rCursor, SwRects & rRects, SwRegionRects aRegion( !bIgnoreVisArea ? pSh->VisArea() : getFrameArea() ); + if (pPieces) + pPieces->clear(); + if (pHoles) + pHoles->clear(); + aRegion.RecordSubtracted(pPieces); if( !pStartPos->GetNode().IsContentNode() || !pStartPos->GetNode().GetContentNode()->getLayoutFrame(this) || ( pStartPos->GetNode() != pEndPos->GetNode() && @@ -2656,6 +2662,7 @@ void SwRootFrame::CalcFrameRects(SwShellCursor const& rCursor, SwRects & rRects, } aRegion.Invert(); + aRegion.RecordSubtracted(pHoles); // what follows cuts holes, not pieces pSt2Pos.reset(); pEnd2Pos.reset(); diff --git a/sw/source/core/text/EnhancedPDFExportHelper.cxx b/sw/source/core/text/EnhancedPDFExportHelper.cxx index c84c1b0b1995..84875715f893 100644 --- a/sw/source/core/text/EnhancedPDFExportHelper.cxx +++ b/sw/source/core/text/EnhancedPDFExportHelper.cxx @@ -30,7 +30,6 @@ #include #include -#include #include #include #include @@ -581,133 +580,98 @@ bool lcl_TryMoveToNonHiddenField(SwEditShell& rShell, const SwTextNode& rNd, con return true; }; +// absorb into rRects[rInto] one rectangle that touches it, nSlack twips apart counting as +// touching, unless their union would reach over one of rHoles +bool lcl_AbsorbOne(SwRects& rRects, size_t& rInto, tools::Long nSlack, const SwRects& rHoles) +{ + const SwRect aGrown(rRects[rInto].Pos() - Point(nSlack, nSlack), + rRects[rInto].SSize() + Size(2 * nSlack, 2 * nSlack)); + for (size_t i = 0; i < rRects.size(); ++i) + { + if (i == rInto || !aGrown.Overlaps(rRects[i])) + continue; + const SwRect aJoined(rRects[rInto].GetUnion(rRects[i])); + if (std::ranges::any_of( + rHoles, [&aJoined](const SwRect& rHole) { return aJoined.Overlaps(rHole); })) + { + continue; + } + rRects[rInto] = aJoined; + rRects.erase(rRects.begin() + i); + if (i < rInto) + --rInto; + return true; + } + return false; +} + +void lcl_JoinTouching(SwRects& rRects, tools::Long nSlack, const SwRects& rHoles) +{ + // the union of two rectangles reaches further than either, so one that has just grown is + // offered the whole vector again, those already passed included + for (size_t i = 0; i < rRects.size(); ++i) + { + while (lcl_AbsorbOne(rRects, i, nSlack, rHoles)) + { + } + } +} + // tdf#157816: try to check if the rectangle contains actual text ::std::vector GetCursorRectsContainingText(SwCursorShell const& rShell) { - ::std::vector ret; - SwRects rects; - rShell.GetLayout()->CalcFrameRects(*rShell.GetCursor_(), rects, SwRootFrame::RectsMode::NoAnchoredFlys); + SwRects region; + SwRects pieces; + SwRects holes; + // the pieces specify which lines each part of the region belongs to + rShell.GetLayout()->CalcFrameRects(*rShell.GetCursor_(), region, + SwRootFrame::RectsMode::NoAnchoredFlys, &pieces, &holes); auto const [pStart, pEnd] = rShell.GetCursor_()->StartEnd(); - for (SwRect const& rRect : rects) + // a ruby's band overlaps its line's text band while lines only abut, so this leaves + // the pieces not overlapping each other; a piece spans its lines whole, so vetoing on + // a hole here would refuse every join + lcl_JoinTouching(pieces, 0, {}); + + ::std::vector ret; + for (const SwRect& rPiece : pieces) { - Point center(rRect.Center()); - SwSpecialPos special; - SwCursorMoveState cms(CursorMoveState::NONE); - cms.m_pSpecialPos = &special; - cms.m_bFieldInfo = true; - // the centre of a one-glyph rectangle falls in that glyph's second half, so the hit - // test below returns the character that contains the point - cms.m_bPosMatchesBounds = true; - // the band of the portion, not the taller band a ruby gives the line - cms.m_bRealHeight = true; - SwPosition pos(rShell.GetDoc()->GetNodes()); - if (rShell.GetLayout()->GetModelPositionForViewPoint(&pos, center, &cms) - && *pStart <= pos && pos <= *pEnd) + SwRects parts; + for (const SwRect& rRect : region) { - SwRect charRect; - std::pair const tmp(center, false); - SwContentFrame const*const pFrame( - pos.nNode.GetNode().GetTextNode()->getLayoutFrame(rShell.GetLayout(), &pos, &tmp)); - if (pFrame->GetCharRect(charRect, pos, &cms, false)) + SwRect aPart(rRect); + aPart.Intersection(rPiece); + if (!aPart.HasArea()) + continue; + + Point center(aPart.Center()); + SwSpecialPos special; + SwCursorMoveState cms(CursorMoveState::NONE); + cms.m_pSpecialPos = &special; + cms.m_bFieldInfo = true; + // the centre of a one-glyph rectangle falls in that glyph's second half, so the + // hit test below returns the character that contains the point + cms.m_bPosMatchesBounds = true; + SwPosition pos(rShell.GetDoc()->GetNodes()); + if (rShell.GetLayout()->GetModelPositionForViewPoint(&pos, center, &cms) + && *pStart <= pos && pos <= *pEnd) { - // a reverse portion in a vertical frame can give a negative offset, which - // no band matches - if (cms.m_aRealHeight.X() >= 0) + SwRect charRect; + std::pair const tmp(center, false); + SwContentFrame const* const pFrame( + pos.GetNode().GetTextNode()->getLayoutFrame(rShell.GetLayout(), &pos, &tmp)); + if (pFrame->GetCharRect(charRect, pos, &cms, false) && aPart.Overlaps(charRect)) { - // the band lies along X, and its extent arrives as a negative height, in - // a vertical frame or a rotated portion but not in the two together - if (cms.m_aRealHeight.Y() > 0) - { - charRect.SetPosY(charRect.Top() + cms.m_aRealHeight.X()); - charRect.Height(cms.m_aRealHeight.Y()); - } - else if (cms.m_aRealHeight.Y() < 0) - { - charRect.SetPosX(charRect.Left() + cms.m_aRealHeight.X()); - charRect.Width(-cms.m_aRealHeight.Y()); - } - } - if (rRect.Overlaps(charRect)) - { - ret.push_back(rRect); + parts.push_back(aPart); } } + // reset stupid static var that may have gotten set now + SwTextCursor::SetRightMargin(false); // WTF is this crap } - // reset stupid static var that may have gotten set now - SwTextCursor::SetRightMargin(false); // WTF is this crap - } - - if (ret.empty()) - return ret; - - // a ruby's band holds no text of the line, so grow the rectangle over the ruby portion - const auto aCoverRuby = [&rShell, &ret](const SwPosition& rPos, sal_Int32 nRubyAt, - const SwRect& rOnItsLine) -> bool { - const SwTextNode* pNode = rPos.GetNode().GetTextNode(); - // a hit test descends into a ruby for the position right after it too, so the hint - // decides whether the link reaches into one - if (!pNode || nRubyAt < 0 || !pNode->GetTextAttrAt(nRubyAt, RES_TXTATR_CJK_RUBY)) - return false; - - const std::pair aPoint(rOnItsLine.Center(), false); - const SwContentFrame* pFrame = pNode->getLayoutFrame(rShell.GetLayout(), &rPos, &aPoint); - if (!pFrame) - return false; - - SwCursorMoveState cms(CursorMoveState::NONE); - cms.m_b2Lines = true; - // CalcFrameRects sets it too - an undersized frame is not scrolled to be hit - cms.m_bNoScroll = true; - // the multi type is the innermost portion's, the portion rectangle the outermost - // one's, so a nested ruby would grow over whatever encloses it - if (!pFrame->GetCharRect(o3tl::temporary(SwRect()), rPos, &cms, false) || !cms.m_p2Lines - || cms.m_p2Lines->nMultiType != MultiPortionType::RUBY - || cms.m_p2Lines->aPortion2.HasArea()) - { - return false; - } - - bool bGrown = false; - for (SwRect& rKept : ret) - { - if (rKept.Overlaps(cms.m_p2Lines->aPortion)) - { - rKept.Union(cms.m_p2Lines->aPortion); - bGrown = true; - } - } - return bGrown; - }; - // either end may reach into a ruby; the rectangle only picks the frame of a node laid - // out more than once, and any rectangle of the link serves for that - const bool bStartGrown = aCoverRuby(*pStart, pStart->GetContentIndex(), ret.front()); - const bool bEndGrown = aCoverRuby(*pEnd, pEnd->GetContentIndex() - 1, ret.back()); - if (!bStartGrown && !bEndGrown) - return ret; - - // rectangles that tile one strip of a line are one annotation - for (bool bMerged = true; bMerged;) - { - bMerged = false; - for (size_t i = 0; !bMerged && i + 1 < ret.size(); ++i) - { - for (size_t j = i + 1; !bMerged && j < ret.size(); ++j) - { - const SwRect aUnion(ret[i].GetUnion(ret[j])); - // the full extent across the line, and no gap along it - const bool bTiles - = (aUnion.Width() == ret[i].Width() && aUnion.Width() == ret[j].Width() - && aUnion.Height() <= ret[i].Height() + ret[j].Height()) - || (aUnion.Height() == ret[i].Height() && aUnion.Height() == ret[j].Height() - && aUnion.Width() <= ret[i].Width() + ret[j].Width()); - if (!bTiles) - continue; - ret[i] = aUnion; - ret.erase(ret.begin() + j); - bMerged = true; - } - } + // what holds no text is gone, so joining the rest keeps a line whole, and a hole + // stops a join that would cover what the selection excludes on purpose + lcl_JoinTouching(parts, 2, holes); + ret.insert(ret.end(), parts.begin(), parts.end()); } return ret; }