From 88ebb3f2f513dbce5a775f50e51ea0117f859c65 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 17 Jun 2022 11:09:43 +1000 Subject: [PATCH] Correctly set transform for QgsHighlight When we are highlighting a feature, we skip the early transformation and instead defer to the layer's renderer to handle this for us. By correctly setting up the render context with the correct transform and transformed map extent, we ensure that the highlight will render correctly even for complex map symbols (eg geometry generators) Fixes #48439 --- src/gui/qgshighlight.cpp | 55 ++++++----- tests/src/python/test_qgshighlight.py | 91 +++++++++++++++++- .../expected_highlight_transform.png | Bin 0 -> 8194 bytes 3 files changed, 117 insertions(+), 29 deletions(-) create mode 100644 tests/testdata/control_images/highlight/expected_highlight_transform/expected_highlight_transform.png diff --git a/src/gui/qgshighlight.cpp b/src/gui/qgshighlight.cpp index edfab451ad5..5c508e0352f 100644 --- a/src/gui/qgshighlight.cpp +++ b/src/gui/qgshighlight.cpp @@ -83,35 +83,21 @@ void QgsHighlight::init() void QgsHighlight::updateTransformedGeometry() { - QgsCoordinateTransform ct = mMapCanvas->mapSettings().layerTransform( mLayer ); - if ( ct.isValid() ) + const QgsCoordinateTransform ct = mMapCanvas->mapSettings().layerTransform( mLayer ); + + // we don't auto-transform if we are highlighting a feature -- the renderer will take care + // of that for us + if ( ct.isValid() && !mGeometry.isNull() ) { // reset to original geometry and transform - if ( !mGeometry.isNull() ) + mGeometry = mOriginalGeometry; + try { - mGeometry = mOriginalGeometry; - try - { - mGeometry.transform( ct ); - } - catch ( QgsCsException & ) - { - QgsDebugMsg( QStringLiteral( "Could not transform highlight geometry to canvas CRS" ) ); - } + mGeometry.transform( ct ); } - else if ( mFeature.hasGeometry() ) + catch ( QgsCsException & ) { - mFeature.setGeometry( mOriginalGeometry ); - QgsGeometry g = mFeature.geometry(); - try - { - g.transform( ct ); - mFeature.setGeometry( g ); - } - catch ( QgsCsException & ) - { - QgsDebugMsg( QStringLiteral( "Could not transform highlight geometry to canvas CRS" ) ); - } + QgsDebugMsg( QStringLiteral( "Could not transform highlight geometry to canvas CRS" ) ); } } updateRect(); @@ -329,6 +315,27 @@ void QgsHighlight::paint( QPainter *p ) return; QgsRenderContext context = createRenderContext(); + const QgsCoordinateTransform layerToCanvasTransform = mMapCanvas->mapSettings().layerTransform( mLayer ); + context.setCoordinateTransform( layerToCanvasTransform ); + QgsRectangle mapExtentInLayerCrs = mMapCanvas->mapSettings().visibleExtent(); + if ( layerToCanvasTransform.isValid() ) + { + QgsCoordinateTransform approxTransform = layerToCanvasTransform; + approxTransform.setBallparkTransformsAreAppropriate( true ); + try + { + mapExtentInLayerCrs = approxTransform.transformBoundingBox( mapExtentInLayerCrs, Qgis::TransformDirection::Reverse ); + } + catch ( QgsCsException & ) + { + QgsDebugMsg( QStringLiteral( "Error transforming canvas extent to layer CRS" ) ); + } + } + if ( !mapExtentInLayerCrs.isFinite() ) + { + return; + } + context.setExtent( mapExtentInLayerCrs ); // Because lower level outlines must be covered by upper level fill color // we render first with temporary opaque color, which is then replaced diff --git a/tests/src/python/test_qgshighlight.py b/tests/src/python/test_qgshighlight.py index 35d1a1b5dc9..96c3d2ed38b 100644 --- a/tests/src/python/test_qgshighlight.py +++ b/tests/src/python/test_qgshighlight.py @@ -17,26 +17,38 @@ import shutil from qgis.PyQt.QtCore import ( QSize, - Qt + Qt, + QDir, + ) from qgis.PyQt.QtGui import ( QColor, QImage, QPainter, - QResizeEvent + QResizeEvent, + QPixmap ) from qgis.core import ( QgsVectorLayer, QgsProject, QgsRectangle, - QgsRenderChecker + QgsRenderChecker, + QgsCoordinateReferenceSystem, + QgsMultiRenderChecker, + QgsGeometryGeneratorSymbolLayer, + QgsFillSymbol, + QgsSingleSymbolRenderer, + QgsSymbol +) +from qgis.gui import ( + QgsHighlight, + QgsMapCanvas ) -from qgis.gui import QgsHighlight from qgis.testing import start_app, unittest from qgis.testing.mocked import get_iface from utilities import unitTestDataPath -start_app() +app = start_app() TEST_DATA_DIR = unitTestDataPath() @@ -48,8 +60,13 @@ class TestQgsHighlight(unittest.TestCase): self.iface.mapCanvas().viewport().resize(400, 400) # For some reason the resizeEvent is not delivered, fake it self.iface.mapCanvas().resizeEvent(QResizeEvent(QSize(400, 400), self.iface.mapCanvas().size())) + self.report = "

Python QgsMapCanvas Tests

\n" def tearDown(self): + report_file_path = "%s/qgistest.html" % QDir.tempPath() + with open(report_file_path, 'a') as report_file: + report_file.write(self.report) + QgsProject.instance().removeAllMapLayers() def runTestForLayer(self, layer, testname): @@ -115,6 +132,70 @@ class TestQgsHighlight(unittest.TestCase): finally: self.iface.mapCanvas().scene().removeItem(highlight) + def test_feature_transformation(self): + poly_shp = os.path.join(TEST_DATA_DIR, 'polys.shp') + layer = QgsVectorLayer(poly_shp, 'Layer', 'ogr') + + sub_symbol = QgsFillSymbol.createSimple({'color': '#8888ff', 'outline_style': 'no'}) + + sym = QgsFillSymbol() + buffer_layer = QgsGeometryGeneratorSymbolLayer.create( + {'geometryModifier': 'buffer($geometry, -0.4)'}) + buffer_layer.setSymbolType(QgsSymbol.Fill) + buffer_layer.setSubSymbol(sub_symbol) + sym.changeSymbolLayer(0, buffer_layer) + layer.setRenderer(QgsSingleSymbolRenderer(sym)) + + canvas = QgsMapCanvas() + canvas.setDestinationCrs(QgsCoordinateReferenceSystem('EPSG:3857')) + canvas.setFrameStyle(0) + canvas.resize(600, 400) + self.assertEqual(canvas.width(), 600) + self.assertEqual(canvas.height(), 400) + + canvas.setLayers([layer]) + canvas.setExtent(QgsRectangle(-11960254, 4247568, -11072454, 4983088)) + canvas.show() + + # need to wait until first redraw can occur (note that we first need to wait till drawing starts!) + while not canvas.isDrawing(): + app.processEvents() + canvas.waitWhileRendering() + + feature = layer.getFeature(1) + self.assertTrue(feature.isValid()) + + highlight = QgsHighlight(canvas, feature, layer) + color = QColor(Qt.red) + highlight.setColor(color) + color.setAlpha(50) + highlight.setFillColor(color) + highlight.show() + highlight.show() + + self.assertTrue(self.canvasImageCheck('highlight_transform', 'highlight_transform', canvas)) + + def canvasImageCheck(self, name, reference_image, canvas): + self.report += "

Render {}

\n".format(name) + temp_dir = QDir.tempPath() + '/' + file_name = temp_dir + 'rendered_' + name + ".png" + + image = QImage(canvas.size(), QImage.Format_ARGB32) + painter = QPainter(image) + canvas.render(painter) + painter.end() + image.save(file_name) + + checker = QgsMultiRenderChecker() + checker.setControlPathPrefix("highlight") + checker.setControlName("expected_" + reference_image) + checker.setRenderedImage(file_name) + checker.setColorTolerance(2) + result = checker.runTest(name, 20) + self.report += checker.report() + print((self.report)) + return result + if __name__ == '__main__': unittest.main() diff --git a/tests/testdata/control_images/highlight/expected_highlight_transform/expected_highlight_transform.png b/tests/testdata/control_images/highlight/expected_highlight_transform/expected_highlight_transform.png new file mode 100644 index 0000000000000000000000000000000000000000..ff06a04f150f3f326c45f1d01c372ceffd63f2db GIT binary patch literal 8194 zcmdsc`9IX(7yp>-LZv2^sO*dovNdQyh^dr4l$}JjVJtIK?^aukJ;q3rWEs2cybDd1 zVeD(gScVWY$o{?FpU30-4}5?7{$M;_cRBambI*OA=Q;P)O*2D&-jlo#2!!A0`nB5- z2-FG!VSB^P30nNrL{q>&9^dQM{t$@3*Zn`XM0tTz5Qtcr(Y4EWg3}kqLf&Mq-v6;o z^+8D%B=sriSwVRu*sDomSA><49||}%4z>gp-D`c6_N7NEqt|SNA(k3MV+3e*+(TYf)aMzLXAbtMRsOBJwromkKyc#+Qwrio@R^|+)M&k%(_-!R_azZL z1>d@03so{;hi%7?)!pXC+;V**H zsId9e-imq~^!JF9rLmoN8$!cA+>otf$HpD93V457`#GU}cz05&$}pv1A70sk>$N+t>9=0kY8HiVHOw%bo zmHkW$L+8Fvb{nr2Z)Dn|xglKQMef>LYy61z`qh?;QF#utOVsaqFt?7A}f9slEmXv0dK_{;mv3$o2)CzU^S8;sym7-@3Q9OJArop`LKSOHK76 z;ceA{0+Jl=Flc1b`lvspW$CxEXvlk3!aL6#{(Bu-(}d&f&+IV?r_6jk5sE|Y&9UqtJ?179r=)M zNo_tZy|c4FUl6CS{PZd19!fGcY^y2D{Hmqgy?D%6lP6hj2Kc+uWx>DJY4?J?Z15<|;OUBmoJ^=OYCyhjgBW#QvfgCX4R-uU?YR-cPRrH1Fi zs8i}KIh>khV>RX@JGWO>Ooo!32dH}@#XLgKq28OU0A!|LSDs6&2wnu0F7K+hdg*EN zXEdvuzNWkU`g#@*hR1!2Fh?7UeBtT>ZeIy@*yIGj%%z>= zt$5V9EP}(}LxV1}U$N68M^eQ#LG*yCHm*5kq(*(CO?M>Y66+>IRta5hvq`namwVwk z_>5!O(h?gVw1=X+$9F=@V~433NCAn+?wh2Uv8%K=7LUeQEpY) z`k0FDC4GJ(VT8e0g!?_VAO~H!6mSHmPa*`LtJ=t0KX|VNITdRbF~G>ulA+vawIXij@~8nZ!7JcGC0trbKCrPla57|L zispnZrt`uQgJ&}6r}iHEdyb655gayGQ7E%ozq_3}+6;y)g7agVyw#lt%;}f%!ZG1n zEA&dPmFT^46)M6Keek9lPG3?qtcV45H!U~BBt`FepHUyzV2zs*5!`Ttg@Vd>@5(U; zG-p}aOoR5DhQD)pArr5F+H(87afTCt8aHvnrBwQ~?#XY@`LFK%f|i}fHGix^aQC67 zp5wMxF_`n~KQa#rB-tN``UiRVf}Brn-1gX#cKPX+_ILW$H4&ak=L3UaWk@sY_tyCk)ZW}Akl=%0R zm8_F`j>_i%Kn(2u{9m-<{nI98*3J@yQ(qsWypl#Mi5efaiuyL)K~ZoBT;)w0dMuVZ zGZAS*#&*&hyoKYqL}ltA8tKQ?5B{}&c7+}GrNRxlvgs|DNMIAe)imthtavC}2i1yg zd@v{)u1Z-PBe_8gAFMAv{M)8p>V{;T?Xm`wrjrhoB@GL`Exmu0aqPdA5^vj*nV3*x zD&)DxNGzw{Oa{qPZ|0EI!M3Y{lFLoz?nZ)peRUzzsWf?_3OD@uRGg_O9h;=4k@B{F z&91^n5y3v3**NjidnlgX#AQN-7-X$|PqIUeb=_}j96W3#qU_DF@GGN+bLEQ|2(1gI zj*qXMjHc&i)&h{Z8Rtem^-VlB0d$yv4nl-hZko<}O_UZKN*9-rhRg$6J7Fe*~DOPf*IGc6QF{-xW z@tMle+h~rmlkmxS#SQvk34Vz z#;+In-y@c2P9?L&&7LihACO~*tVH5Ro_5_I50OS~Q$zih>PRp2yo9Vodix8ECUXt| zH6{zXztv$bzRBEv&pPCO4;uN@ZtNu9&k5UA4~Sx z$y8ryIR%vV7qY5-G4ixAtCtXV4zs0e|4p6@BzBlM7H2j!Igfa+1GiVL8g5gZ3UwGu zpCQQCv8Ul1ALx!$G#fs{ir)s&_2TaG)uG>Uron=^=7>v{YzNyG$Ev1~7d2CacX>b) zJ51@}*4mWV?z-M$eGRaNC^T}Xmht6jBRAsDDw}1=&lz?YIIKqZ(ZG(Lq4GQyuME<$ zSgev8*Zr}sLQQ?_4w@rDx@eGEFP$liU~_+L>m%|Fj|3j%!UG>KNBP_y+4N+b&^z_F zyPT}K{47SQC^&hBjQ!M0h>-j5$P>i>*CK~tW`r^)B7s|{?cN+js73U@PeV&|b&o*h|$fi^gwA9J3o&W~y ze@zGd(0Q}9q*w=SiEV^B+Kq;H;D@S8S-iqxP(JMATcMMIAShmP?&l4axI5i3Yk9)PsqXaR2|5V}a%=|qJ5dPb9X{JlTZAv;d&Vep7o zfV(XadSS6a7d38;W{={>ap)=Q)~q$vuGuh--9&&-&CoUa<~-gYn|0`8QX`c3u?}^R zUcY5b#(ILp==G_;Z>4JuUR0bbg2)uEO`jV2T+HJA7y|~%)<*Q`iDGKO=X3DsF}kv# zFjSfYHnK0a=Obdz?ooZ!f1k}Yp5-(?Nc>C#3Q54!gSOMS<{FE908D3$vg1xQR7!8H zC5l4Dfk_9Ra8Szl=D@9x5Y(Af(<)O1UI8K>ZNwJMsV7>uhI=?>gXV~e0)Ex=K{B$? z$_BKTpvVKWT_x*+q`I; zAok}Qom6p6BCx%Y7+T9_GgR-o0B)z+;|C>wp7H@W3?2~2g!>TCpIE#amvGHz3v7-p zb+M(j{&bA^TkZy6lX2G99U6Hx?V>_zSbrMObo?&x$=$`JeX>!QD$gR(9#YiM zRKw-hV+xME34qHy9glBmw+5#9J=4s8plYd}CjYaM9rmJcETdf)6#=%kr2HqE>+}_4 z^CMOwaf$LKiVzJfB6MZP=V6Df53ukSz@50EQamAWb${LLwf3HkMHHCezYn;AH3vF8 z2MP{^{QU(ai}Ao?26J_Yw7|S(iiCE{?k4Kkt zgscvZ&`3t;P!Iw%h{ImC{_A5T%Ps~8I{hPW+jUSLU|(FTv(H;abc}C7BM&bN!@Al* zo-h-mZA>QOdEf^0zms^ly&amf@iKyWgTWDS0Yp;;&AJ8_JN9XsYAy?^mD>2=t402G zT8}vx#byc;Beskij>-?h!znC(OK_OJdeF#tc3AereH78!^sUAJ`sMgw648@L8N`8~ z2)m|t5`X~odQv7zT>;Yl0noHRqvC;n{CP|wcDj2uGoFGs0Xl6|p3A-%=t z<^|DSk)*qRvBMIr7+?O;6H3+UIeZ~OvgqU2clt(?DNbPP*C#gQzYAI2d6y@j>!4G~ zI*65kMy`Mv*5z`VYhU({%a7GfI=*k1#Ogo)z7I51W0)y7{K@-E9Pd(yEiB z>Z*B)PiC@AMLLN9fkw`mW~Als{mawY0&TfBIjCt}UB==C^fGRPiI=q0KE)^OkW+|p zsICF|DXv*qYck4t6HRqun*_Als~=$2Rd$%u9L)p|x4sPm4BK?XpI+7)0Mq?;&xx<} z*GQyUHpZACtEZjUR}$pSIQ%CTbUm7Gr~g(=2S?J?@*$%^3b1B-w!Q0)%I9H0FcT1TVCku3>F5nUV*yUj$vp17JONy_4lwq`K_`59CaU*>5?a3VErWDeRT85h`v4HB z#VJB7)Syt=E6Ab{UI!Jp;drZkbJd8d6~_^!XKC4w`Ss5Ymc3gVNWoJU7wO;S@h!@& zNd#zF3Mtb%1Ei2->{B7DqkTUz?fyQiGD0H3ZX;7Rve5kn@<1FQ)uXi`l-E@B8>mKY z%PZ-rcoP7hU-HyWMjJ2=zmZUENWb*gafP)&&8e80qWNQ`0-BQNAMw+Uu)|EymFTCK3)rWtZ;W9ecGKeRY*qN$r-TZI=sEm4Hle)(;%*|vG(EZ-St{uiY%v9$N za$ir1Rsl=or;=0y<}OH-$k@1fSp=lTq#!BZCJ}B^rldZWP+QR*C(xC@pP54pT93iC zI&T?DTH3{>Gs*KFBA6=-%|_k6p2>2JvP+rg-e)h2y&Xk+t+b=k*#$e^?Tw-fs{yPF zpSCQTbr+JBnjer(-rnk4J}=-dpo6%seaPyKb7wl8*qt$FA)1BhQxMN7|=?u8ghnZfJWLGIh6q;`_RqzX3o z^Hu7^4ZrnCAh3Ji-`qqO)|TwY>ZbNq>f<#FYRk|9@p+g?0pnP%_J`N=w<|Q6q{B!F{ zNMHa@>G}Mbl{T=4Ic~VHc<8zmDA#QN4vi&MIE6AayLvl!7pj?LcX=pZ%b=}~r0(|p zk|}iSB>=`5X_&Q47^w!(y^k^Y(fd)R1b`eKel5v!9yc;9&;qI%+0>(!WnwsvNstxM zCmSl2Kt(Q@05xr{;qov961&ShD1DaF(wJ&Oz{c{^7S;*tA|lu=;m04P&g$y;QmUB_ zE3wZbU8#X3BIDVaDqW4ZZhOS)<~MM2-6BFQTqC|IT-6$wXeJ$R96V%ov+KPP<(Q>J z&JG#@c5Y0?E%)^$hA@p|vBhVLh&=gLadsO^W=0FDmZh_2@+{qG?`h(W54qv)M^s!T zGPmsfCZy-CDqi~MBI;kBeUXQ)x~>5~W_FsK|EumrOYyg3Xexb((XD4eg|(r4X` zxj7Qw!oxJ75)Y_u=Fp$;0oYmw9`+?LP*T1VC3D!xXQS23tYKz^aR|!yZDq>Hsc`FJ zRF+Gd->?Tz^Y39RJ#;8uaBohT^4^G))Ad0om?5!rGAH}*ynuw!bh}ai?pz5?#r^93 z{ro!#?&q>(5f-K3KxnV|x0wGz25=@y1*ms{9ri~wJX{p*#)5?f8^ve=M3lMe$i_v_LK(PBT(cz4x93y%tN5<>e9j1 z?iqy+P))Javj!s0`A8N57d59m}G1@d1F@xiIU;*5|{l#Gq?LADBs5$1xa>jMwiEU8Ch*<{kwz) zu-lzadGb}iOPs%loXb0{21?7I&}XT~6auLyK$O@B)zq+xZ=ULr{h2=l6MWf$c2a0` zd@RP!xjeD3F#j=DSL85`BPx|gC>|McL33FMh`FKTAGo;%wC+DsgjEL6mX5ObTk{#f z=bD>P?;rgyJc48g+R{vsmKNJtdFQ;bLNgbB#UoS_{FrgLOAD;>2;2^xt>#y$fmycC z3tYcj%kM)JN0|UvEDtQ!ricX38=_){UZ)=k{`-d;PJD?*RQ+j_@6lKcC*}AGM~_pi z%RaJr-4&pGGOmlGbn^xkX?$luTTyDqrO;RF0{<%Uz=+ z$JPvP$4q8s`iA1X2MZ~Sc;P?;+#7EuR|mBLS#?^AY>tH3&UXrhMk3|>@o!BJFQJ}m zg2|)pVZPkD?b{6545#MT`P!kNyl({ZS|_3!(2}t*7-xsNx1V#8a>~kIEs?=x ziGRiiYAXHGMc5=IjTG>J03u%8-SXTS!DoG=RitFStwUCNZSJbi&&^LY9JkKy@4-9i zlwFwbmn?Nl7CiupRcs2RZcqn)N^j;r=OqCk%4JtS$9ZAcd9Ih{MGOZEPpO`yM5{|F zh9R9#=Ys4|bL!nj?f`y#9Q1>i4Wi4K?^Ai}7d;yHBP{Hd-GwGKwR7s zGx2U+qm>He0Rlfw2)UA^;4dQsD)1ZuwFcrL3*}jZo6VU`TcX$378MW*jQO_wQe+2> zv|k%_JZRCL%Z!^il&NnnM|riar-(KAdqa3JMGP`|DQht|GzwO b?_j!5n4@efm*0U$eTb31*|owej!*s%Pf?&E literal 0 HcmV?d00001