From 194e7ce17ae7ca278c12d03bc25684b7437f9785 Mon Sep 17 00:00:00 2001 From: Jan-Marek Glogowski Date: Thu, 10 Oct 2019 14:48:22 +0200 Subject: tdf#40534 correctly match page with memory slab LO has a page manager to match system memory backbuffers with graphics memory on DX accelerated Windows. Internally this uses an other rectangle implementation, the SurfaceRect, which had some great comments like: // a size of [0,0] therefore denotes a one-by-one rectangle. In commit 230dbe2e43f3ee2cd285f9cdfe0d57e1ca08b8fe ("#144866# Add one pixel border around textures, a bunch of drivers clobber those with dirt), the allocation was increased by a pixel border, but this doesn't work correctly, because now an allocation of the page size wouldn't fit anymore into a page, because the pages size is decreased before comparison. In the end the mixup suffered from hard to handle off-by-one problems. This patch fixes the bug, but eventually SurfaceRect should be replaced by an extended basegfx::B2IBox. But since B2IBox uses two ranges, instead of a point and a size, it would need a lot of conversations to I2Point and I2Size objects with the current Page::insert algorithm. Change-Id: Ia725b4f8ed4fb270f2eb3734e492062bc7f13793 Reviewed-on: https://gerrit.libreoffice.org/80628 Tested-by: Jenkins Reviewed-by: Jan-Marek Glogowski --- canvas/source/tools/page.cxx | 2 +- canvas/source/tools/surfacerect.hxx | 74 ++++++++++++------------------------- 2 files changed, 24 insertions(+), 52 deletions(-) diff --git a/canvas/source/tools/page.cxx b/canvas/source/tools/page.cxx index 4d83851d806a..78f9cd3aa671 100644 --- a/canvas/source/tools/page.cxx +++ b/canvas/source/tools/page.cxx @@ -118,7 +118,7 @@ namespace canvas // the rectangle passed as argument has a valid // location if and only if there's no intersection // with existing areas. - SurfaceRect aBoundary(mpRenderModule->getPageSize()-basegfx::B2IVector(1,1)); + SurfaceRect aBoundary(mpRenderModule->getPageSize()); if( !r.inside(aBoundary) ) return false; diff --git a/canvas/source/tools/surfacerect.hxx b/canvas/source/tools/surfacerect.hxx index e0cd0fc023ba..ec73342ea799 100644 --- a/canvas/source/tools/surfacerect.hxx +++ b/canvas/source/tools/surfacerect.hxx @@ -25,6 +25,16 @@ namespace canvas { + /** + * This implements some equivalent to basegfx::B2IBox, but instead of two + * BasicBox ranges, it uses a position and a size. maPos and maSize could + * be replaced by: + * - B2IPoint(getMinX(), getMinY()) and + * - B2ISize(getMaxX()-getMinX(), getMaxY()-getMinY()) + * + * The current allocation algorithm uses size and pos a lot. Not sure how + * time-critical any of this code is and if that would be a problem. + */ struct SurfaceRect { ::basegfx::B2IPoint maPos; @@ -36,22 +46,12 @@ namespace canvas { } - // coordinates contained in this rectangle are - // constrained to the following rules: - // 1) p.x >= pos.x - // 2) p.x <= pos.x+size.x - // 3) p.y >= pos.y - // 4) p.y <= pos.y+size.y - // in other words, 'size' means the number of pixels - // this rectangle encloses plus one. for example with pos[0,0] - // and size[512,512], p[512,512] would return inside. - // a size of [0,0] therefore denotes a one-by-one rectangle. bool pointInside( sal_Int32 px, sal_Int32 py ) const { const sal_Int32 x1(maPos.getX()); const sal_Int32 y1(maPos.getY()); - const sal_Int32 x2(maPos.getX()+maSize.getX()); - const sal_Int32 y2(maPos.getY()+maSize.getY()); + const sal_Int32 x2(x1 + maSize.getX()); + const sal_Int32 y2(y1 + maSize.getY()); if(px < x1) return false; if(px >= x2) return false; if(py < y1) return false; @@ -59,58 +59,30 @@ namespace canvas return true; } - /// returns true if the horizontal line intersects the rect. - bool hLineIntersect( sal_Int32 lx1, sal_Int32 lx2, sal_Int32 ly ) const - { - const sal_Int32 x1(maPos.getX()); - const sal_Int32 y1(maPos.getY()); - const sal_Int32 x2(maPos.getX()+maSize.getX()); - const sal_Int32 y2(maPos.getY()+maSize.getY()); - if(ly < y1) return false; - if(ly >= y2) return false; - if((lx1 < x1) && (lx2 < x1)) return false; - if((lx1 >= x2) && (lx2 >= x2)) return false; - return true; - } - - /// returns true if the vertical line intersects the rect. - bool vLineIntersect( sal_Int32 lx, sal_Int32 ly1, sal_Int32 ly2 ) const - { - const sal_Int32 x1(maPos.getX()); - const sal_Int32 y1(maPos.getY()); - const sal_Int32 x2(maPos.getX()+maSize.getX()); - const sal_Int32 y2(maPos.getY()+maSize.getY()); - if(lx < x1) return false; - if(lx >= x2) return false; - if((ly1 < y1) && (ly2 < y1)) return false; - if((ly1 >= y2) && (ly2 >= y2)) return false; - return true; - } - /// returns true if the passed rect intersects this one. bool intersection( const SurfaceRect& r ) const { const sal_Int32 x1(maPos.getX()); const sal_Int32 y1(maPos.getY()); - const sal_Int32 x2(maPos.getX()+maSize.getX()); - const sal_Int32 y2(maPos.getY()+maSize.getY()); - if(r.hLineIntersect(x1,x2,y1)) return true; - if(r.hLineIntersect(x1,x2,y2)) return true; - if(r.vLineIntersect(x1,y1,y2)) return true; - if(r.vLineIntersect(x2,y1,y2)) return true; - return false; + const sal_Int32 x1w(x1 + maSize.getX() - 1); + const sal_Int32 y1h(y1 + maSize.getY() - 1); + + const sal_Int32 x2(r.maPos.getX()); + const sal_Int32 y2(r.maPos.getY()); + const sal_Int32 x2w(x2 + r.maSize.getX() - 1); + const sal_Int32 y2h(y2 + r.maSize.getY() - 1); + + return !((x1w < x2) || (x2w < x1) || (y1h < y2) || (y2h < y1)); } bool inside( const SurfaceRect& r ) const { const sal_Int32 x1(maPos.getX()); const sal_Int32 y1(maPos.getY()); - const sal_Int32 x2(maPos.getX()+maSize.getX()); - const sal_Int32 y2(maPos.getY()+maSize.getY()); + const sal_Int32 x2(x1 + maSize.getX() - 1); + const sal_Int32 y2(y1 + maSize.getY() - 1); if(!(r.pointInside(x1,y1))) return false; - if(!(r.pointInside(x2,y1))) return false; if(!(r.pointInside(x2,y2))) return false; - if(!(r.pointInside(x1,y2))) return false; return true; } }; -- cgit