<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"><meta name="Generator" content="Microsoft Word 15 (filtered medium)"><style></style></head><body lang="EN-US" link="#0563C1" vlink="#954F72"><div class="WordSection1">Hi, (tried sending this email, and got a rejection cause the attachment was too large, so resending it with a smaller attachment). This is in relation to the discussion I had yesterday with Miklos & Thorsten about a fix I pushed to gerrit. The patch rejected by Miklos can be found here è <a href="https://gerrit.libreoffice.org/#/c/5272/1/svx/source/unodraw/unoshap2.cxx">https://gerrit.libreoffice.org/#/c/5272/1/svx/source/unodraw/unoshap2.cxx</a> The discussion we had yesterday on IRC was this:<img border="0" width="1469" height="453" id="_x0000_i1025" src="cid:image002.png@01CE935D.0E79ADA0" alt="Miklos: Why is this necessary? You could avoid introducing a new UNO property by just adding the MirroredX/Y keys to the existing CustomShapeGeometry, right? Miklos: Just like you already use that API in the test-case. Adam: I did not add a *new* UNO property ... I used an existing one. what exactly is the problem ? Miklos: hm, but why it's necessary to touch svx core when you could just insert the mirror property to the existing CustomShapeGeometry? Miklos: I mean the "in core, let's not have multiple ways to achieve the same thing" problem. Adam: but the same function (SetMirroredX) is called a few lines later, so I am not sure why it is a problem (http://opengrok.libreoffice.org/xref/core/svx/source/unodraw/unoshap2.cxx#1941) Miklos: Thorsten: any opinion about the svx part of https://gerrit.libreoffice.org/#/c/5272/ ? i think it's already possible the put the mirror option to the CustomShapeGeometry, Miklos: so i think it's not necessary to touch svx just for the sake of the VML import filter. Thorsten: are you sure? Miklos: the same change uses that api for reading "is mirrored?" in the testcase. so unless that api is read-only, i assume that's possible. (but i was asking :) ) Thorsten: I can find only one call to SetMirrored[XY] in svx uno code Thorsten: gah. fun. EnhancedCustomShape2d.cxx handles it separately Thorsten: Adam- ^^ what Miklos says Thorsten: Adam- does the MirroredY / MirroredX property not work for you otherwise? svx/source/customshapes/EnhancedCustomShape2d.cxx seems to handle it already? "><div> I have asked our developer that made the fix to explain why he did it, and he sent me the attached explanation. Thorsten - Could you give me your thoughts about it? I would be happy to convey him any rejects you have about what he said. Best, Adam Fyne Office: +972-77-517-5008 <a href="http://www.twitter.com/cloudoninc" target="_blank">Twitter</a> | <a href="http://www.linkedin.com/company/cloudon" target="_blank">LinkedIn</a> | <a href="http://www.facebook.com/cloudoninc" target="_blank">Facebook</a> | <a href="http://www.cloudon.com/" target="_blank">Blog</a> </div></div></body></html>