From a5c68eaa1ef94653bc2f62a55ec6fed68de61c78 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Mon, 21 Dec 2020 17:55:02 +0100 Subject: sw: fix layout loop due to a keep-with-next vs 100% image height conflict The document in question has a landscape page, the last paragraph has an image where the width is 100% and the height is set to keep the ratio (which 1:1). This means that SwFlyFreeFrame::CheckClip() will scale the image down, so the image fits the page (and give up that 100% width was requested). The next problem is that this image has a wrap type set to none, so not only the total height of the page is taken, but also no content is allowed on the left/right side, effectively taking the entire body frame. Combine this with a previous paragraph, which is a heading, so keep-with-next is set for it. Layout loops because keep-with-next and take-the-entire-page are conflicting requirements and we used to have no explicit code to relax one of them when both are present: SwFlowFrame::MoveBwd: frame is 4, old upper is 10, new upper is 2 SwFlowFrame::MoveFwd: frame is 4, old upper is 2, new upper is 10 SwFlowFrame::MoveBwd: frame is 4, old upper is 10, new upper is 2 SwFlowFrame::MoveFwd: frame is 4, old upper is 2, new upper is 10 SwFlowFrame::MoveBwd: loop control, frame is 4, old upper is 10, new upper would be 2, but not moving there Fix the problem by giving up keep-with-next to stop the loop, mostly because we have to give up one of the requirements and Word resolves the conflict this way. Change-Id: I7da1ebcff9302cd144887f63efb94e0b2b2be8fd Reviewed-on: https://gerrit.libreoffice.org/c/core/+/108119 Reviewed-by: Miklos Vajna Tested-by: Jenkins (cherry picked from commit 160db4bf0fc391b2ded635d0bd998d5352541742) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/108011 Reviewed-by: Xisco Fauli --- .../core/layout/data/keepwithnext-fullheight.fodt | 42 ++++++++++++++++ sw/qa/core/layout/layout.cxx | 18 +++++++ sw/source/core/layout/flowfrm.cxx | 58 +++++++++++++++++++++- 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 sw/qa/core/layout/data/keepwithnext-fullheight.fodt diff --git a/sw/qa/core/layout/data/keepwithnext-fullheight.fodt b/sw/qa/core/layout/data/keepwithnext-fullheight.fodt new file mode 100644 index 000000000000..b33dd80e719e --- /dev/null +++ b/sw/qa/core/layout/data/keepwithnext-fullheight.fodt @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + Landscape page + Heading + iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAQAAAAAYLlVAAAABGdBTUEAALGPC/xhBQAAAAFz + UkdCAK7OHOkAAAAgY0hSTQAAeiYAAICEAAD6AAAAgOgAAHUwAADqYAAAOpgAABdwnLpRPAAA + AAJiS0dEAACqjSMyAAAACW9GRnMAAAAGAAAAAAAMc1XTAAAACXBIWXMAAA3XAAAN1wFCKJt4 + AAAACXZwQWcAAABMAAAAQACdMTgbAAABzUlEQVRo3u3ZPU/CQBjA8X+Jxs3ESUDj4iK+LA5+ + BBfjqBE1cXB2MlFAEqMgxvhNNL4sLsK3UPQL6ObkoAETz+FKW2mxCPRYnucWUu76/OC59C49 + cGOCKqrD9kHRc6ddPv7oW2WCwMh0nF63Myz7Tm8hPTNu0pgHMER3scepTbgK6enJNND83RLn + /878yRaPmgBZFDuMsNLeWB9gmFQHP77MIg9gsYciR50NFKvtjIy10yk84pSZA7DYpwR8scmF + QQCMuoQMpzbh0iAARrlnVn90CWHTsZcAiHPPdINQAuqsc2MQAAnKDUKWEhZ10twaBEDSJWQo + YlFj7S9CzwEegkXWIbQsRAQASFJhpplwbRAACS+hANRJBxMiAkDcJeQ4sQkBhYgMoJ+Ozlwo + 2YQ7AJ6CRxyiUGnVy3hVKb0Af9v7hUG2Wy9TEQCUelFTDULB2S+YKYGOMcpM6UIccOQnRA6A + cSp6ibfI+wkGADBGpTEd8xz1AaAfTQ7huA8AvUw5hVjuA0D/C5OaMN8XACRZ8F0zCggKAQhA + AAIQgAAEIAABCEAAAhCAAAQgAAH4zg3feY4w3Xs44M5+oW0qvCWoGcvaIlM3x/f/ab+O738A + hOCNQr34oD4AAAAldEVYdGNyZWF0ZS1kYXRlADIwMTAtMTItMjBUMTc6MDg6MzYrMDE6MDB6 + 5RscAAAAJXRFWHRtb2RpZnktZGF0ZQAyMDEwLTEyLTIwVDE3OjA4OjM3KzAxOjAwgyNmnAAA + AABJRU5ErkJggg== + + + + diff --git a/sw/qa/core/layout/layout.cxx b/sw/qa/core/layout/layout.cxx index 76445faa4cfe..e9f1d22bd70b 100644 --- a/sw/qa/core/layout/layout.cxx +++ b/sw/qa/core/layout/layout.cxx @@ -242,6 +242,24 @@ CPPUNIT_TEST_FIXTURE(SwCoreLayoutTest, testBtlrNestedCell) CPPUNIT_ASSERT_GREATEREQUAL(nFrameBottom, aPaintArea.Bottom()); } +CPPUNIT_TEST_FIXTURE(SwCoreLayoutTest, testKeepwithnextFullheight) +{ + // The document has a heading (keep with next) and a full-page image in the next paragraph, i.e. + // conflicting requirements. + // Without the accompanying fix in place, this test would have failed with a layout loop in + // SwEditShell::CalcLayout(). + load(DATA_DIRECTORY, "keepwithnext-fullheight.fodt"); + + xmlDocUniquePtr pXmlDoc = parseLayoutDump(); + CPPUNIT_ASSERT(pXmlDoc); + // Make sure the document has 2 pages. + assertXPath(pXmlDoc, "//page", 2); + // Heading stays on page 1 to avoid a layout loop. + assertXPathContent(pXmlDoc, "//page[1]/body/txt[2]", "Heading"); + // Image stays on page 2. + assertXPath(pXmlDoc, "//page[2]/body/txt/anchored/fly", 1); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx index 7b91a1bc74cc..aeeb9df158cf 100644 --- a/sw/source/core/layout/flowfrm.cxx +++ b/sw/source/core/layout/flowfrm.cxx @@ -176,6 +176,61 @@ void SwFlowFrame::CheckKeep() pPre->InvalidatePos(); } +namespace +{ +/** + * Determines if the next content frame after rThis will require the full area of the parent body + * frame. + */ +bool IsNextContentFullPage(const SwFrame& rThis) +{ + const SwFrame* pNext = rThis.FindNextCnt(); + if (!pNext) + { + return false; + } + + const SwSortedObjs* pNextDrawObjs = pNext->GetDrawObjs(); + if (!pNextDrawObjs || !pNextDrawObjs->size()) + { + return false; + } + + for (const auto& pDrawObj : *pNextDrawObjs) + { + if (!pDrawObj) + { + continue; + } + + SwTwips nDrawObjHeight = pDrawObj->GetObjRectWithSpaces().Height(); + const SwPageFrame* pPageFrame = pDrawObj->GetPageFrame(); + if (!pPageFrame) + { + continue; + } + + SwTwips nBodyHeight = pPageFrame->GetLower()->getFrameArea().Height(); + if (nDrawObjHeight < nBodyHeight) + { + continue; + } + + const SwFormatSurround& rSurround = pDrawObj->GetFrameFormat().GetSurround(); + if (rSurround.GetSurround() != text::WrapTextMode_NONE) + { + continue; + } + + // At this point the height of the draw object will use all the vertical available space, + // and also no wrapping will be performed, so all horizontal space will be taken as well. + return true; + } + + return false; +} +} + bool SwFlowFrame::IsKeep(SvxFormatKeepItem const& rKeep, SvxFormatBreakItem const& rBreak, bool const bCheckIfLastRowShouldKeep) const @@ -186,10 +241,11 @@ bool SwFlowFrame::IsKeep(SvxFormatKeepItem const& rKeep, // 3. If bBreakCheck is set to true, this function only checks // if there are any break after attributes set at rAttrs // or break before attributes set for the next content (or next table) + // 4. Keep is ignored if the next frame will require its own page. bool bKeep = bCheckIfLastRowShouldKeep || ( !m_rThis.IsInFootnote() && ( !m_rThis.IsInTab() || m_rThis.IsTabFrame() ) && - rKeep.GetValue() ); + rKeep.GetValue() && !IsNextContentFullPage(m_rThis)); OSL_ENSURE( !bCheckIfLastRowShouldKeep || m_rThis.IsTabFrame(), "IsKeep with bCheckIfLastRowShouldKeep should only be used for tabfrms" ); -- cgit