From 7f1a2f8fa2ce9ddf15f0e32b9faa01d84b60a8bb Mon Sep 17 00:00:00 2001 From: Jonathan Clark Date: Tue, 12 May 2026 07:57:07 -0600 Subject: [PATCH] tdf#124987 sw: Fixed off-by-one error in draw obj anchor clipping rect Since tdf#91260, Writer clips drawing objects to pages by checking whether their anchors are contained by a bounding rectangle. The code that calculated this rectangle had an off-by-one error causing the clipping rectangle to always be exactly 1 twip shorter and narrower than the actual page frame rectangle. As a result, anything with an anchor position on the right or bottom edge of the screen would fail to render - including any objects anchored to an RTL page. Regression from commit 689cead9e0837dc932e3a4cd765f7d319b529018 Author: Michael Stahl Date: Tue Dec 6 22:59:40 2016 +0100 tdf#91260 svx, sw: don't paint off-page part of drawing object Change-Id: Ife6c817f0689ca2b31d22df31f740c30ab548c8c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/205114 Reviewed-by: Jonathan Clark Tested-by: Jenkins --- include/vcl/canvastools.hxx | 1 + sw/source/core/view/vdraw.cxx | 6 ++- vcl/CppunitTest_vcl_text.mk | 1 + vcl/qa/cppunit/canvastoolstest.cxx | 60 ++++++++++++++++++++++++++++++ vcl/source/helper/canvastools.cxx | 13 +++++++ 5 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 vcl/qa/cppunit/canvastoolstest.cxx diff --git a/include/vcl/canvastools.hxx b/include/vcl/canvastools.hxx index 29231610cba2..b738a36b748b 100644 --- a/include/vcl/canvastools.hxx +++ b/include/vcl/canvastools.hxx @@ -161,6 +161,7 @@ namespace vcl::unotools tools::Rectangle VCL_DLLPUBLIC rectangleFromB2IRectangle( const basegfx::B2IRange& ); basegfx::B2IPoint VCL_DLLPUBLIC b2IPointFromPoint(const Point&); basegfx::B2IRectangle VCL_DLLPUBLIC b2IRectangleFromRectangle(const tools::Rectangle&); + basegfx::B2IRectangle VCL_DLLPUBLIC b2IRectangleFromClosedRectangle(const tools::Rectangle&); } diff --git a/sw/source/core/view/vdraw.cxx b/sw/source/core/view/vdraw.cxx index 9477b51f768d..9ca184c0942a 100644 --- a/sw/source/core/view/vdraw.cxx +++ b/sw/source/core/view/vdraw.cxx @@ -136,7 +136,11 @@ void SwViewShellImp::PaintLayer( const SdrLayerID _nLayerID, } pOutDev->Push( vcl::PushFlags::LINECOLOR ); - basegfx::B2IRectangle const pageFrame = vcl::unotools::b2IRectangleFromRectangle(rPageFrame.getFrameArea().SVRect()); + + // tdf#124987: Writer considers frame rectangles closed ranges, and may position anchors + // at the edges of the rectangle. Convert this to B2IRectangle as a closed range, not open. + basegfx::B2IRectangle const pageFrame + = vcl::unotools::b2IRectangleFromClosedRectangle(rPageFrame.getFrameArea().SVRect()); GetPageView()->DrawLayer(_nLayerID, pOutDev, pRedirector, aPaintRect.SVRect(), &pageFrame); pOutDev->Pop(); diff --git a/vcl/CppunitTest_vcl_text.mk b/vcl/CppunitTest_vcl_text.mk index d829c3fc24b5..668f64affd6d 100644 --- a/vcl/CppunitTest_vcl_text.mk +++ b/vcl/CppunitTest_vcl_text.mk @@ -16,6 +16,7 @@ $(eval $(call gb_CppunitTest_set_include,vcl_text,\ $(eval $(call gb_CppunitTest_add_exception_objects,vcl_text, \ vcl/qa/cppunit/canvasbitmaptest \ + vcl/qa/cppunit/canvastoolstest \ vcl/qa/cppunit/complextext \ vcl/qa/cppunit/justificationdata \ vcl/qa/cppunit/text \ diff --git a/vcl/qa/cppunit/canvastoolstest.cxx b/vcl/qa/cppunit/canvastoolstest.cxx new file mode 100644 index 000000000000..02b676c7c2e7 --- /dev/null +++ b/vcl/qa/cppunit/canvastoolstest.cxx @@ -0,0 +1,60 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + * + * This file incorporates work covered by the following license notice: + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed + * with this work for additional information regarding copyright + * ownership. The ASF licenses this file to you under the Apache + * License, Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of + * the License at http://www.apache.org/licenses/LICENSE-2.0 . + */ + +#include +#include + +using namespace ::com::sun::star; +using namespace vcl::unotools; + +namespace +{ +class CanvasToolsTest : public CppUnit::TestFixture +{ +public: + void testB2IRectangleFromClosedRectangle(); + + CPPUNIT_TEST_SUITE(CanvasToolsTest); + CPPUNIT_TEST(testB2IRectangleFromClosedRectangle); + CPPUNIT_TEST_SUITE_END(); +}; + +void CanvasToolsTest::testB2IRectangleFromClosedRectangle() +{ + // tdf#124987: Characterization test for a use that was causing an off-by-one + // error when clipping draw objects to the page containing the anchor + + tools::Rectangle aRect{ /*nLeft*/ 100, /*nTop*/ 100, /*nRight*/ 200, /*nBottom*/ 300 }; + + // With the original code, the simulated RTL anchor would be considered off-screen + auto aBadPageFrame = b2IRectangleFromRectangle(aRect); + CPPUNIT_ASSERT_EQUAL(sal_Int32(200), aBadPageFrame.getMaxX()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(300), aBadPageFrame.getMaxY()); + + // With the new code, the simulated RTL anchor would be considered on-screen + auto aPageFrame = b2IRectangleFromClosedRectangle(aRect); + CPPUNIT_ASSERT_EQUAL(sal_Int32(201), aPageFrame.getMaxX()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(301), aPageFrame.getMaxY()); +} + +} // namespace + +CPPUNIT_TEST_SUITE_REGISTRATION(CanvasToolsTest); + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/source/helper/canvastools.cxx b/vcl/source/helper/canvastools.cxx index 7c9bec07cf4c..3ee1e3c9d099 100644 --- a/vcl/source/helper/canvastools.cxx +++ b/vcl/source/helper/canvastools.cxx @@ -338,6 +338,19 @@ namespace vcl::unotools rRect.IsHeightEmpty() ? rRect.Top() : rRect.Bottom() ); } + basegfx::B2IRectangle b2IRectangleFromClosedRectangle(tools::Rectangle const& rRect) + { + // although B2IRange internally has separate height/width emptiness, it doesn't + // expose any API to let us set them separately, so just do the best we can. + if (rRect.IsWidthEmpty() && rRect.IsHeightEmpty()) + { + return basegfx::B2IRange(basegfx::B2ITuple(rRect.Left(), rRect.Top())); + } + + return basegfx::B2IRange(rRect.Left(), rRect.Top(), rRect.Left() + rRect.GetWidth(), + rRect.Top() + rRect.GetHeight()); + } + basegfx::B2DSize b2DSizeFromSize(const Size& rSize) { return basegfx::B2DSize(rSize.Width(), rSize.Height());