Re: Fix docx import of wrap 'Square' for groups

[Miklos Vajna <vmiklos@xxxxxxxxxxxxx>]

Hi Regina,

On Tue, Apr 20, 2021 at 06:30:48PM +0200, Regina Henschel <rb.henschel@xxxxxxxxxxx> wrote:
> I have adapted some tests now and added some new ones.
> 
> Do you have some time to review the patch?
> https://gerrit.libreoffice.org/c/core/+/114193

Please rebase it, sadly it conflicts with current master. Otherwise, if
you would like to have a review by somebody, you can add them as
reviewer. I try to keep my gerrit dashboard clean. :-)

> My patch excludes rotated groups from scaling by
>          if (m_pImpl->isXSizeValid())
>              aSize.Width = m_pImpl->getXSize();
>          if (m_pImpl->isYSizeValis())
>              aSize.Height = m_pImpl->getYSize();
> 	 m_xShape->setSize(aSize);
> in
> case NS_ooxml::LN_shape:
> in GraphicImport::lcl_attribute()
> and still allows scaling of unrotated groups like that from
> testGroupShapeRotation in GraphicImport.cxx
> 
> But I wonder, why such scaling is needed at all. When the shape is inserted
> by Shape::createAndInsert in oox/source/drawingml/shape.cxx,
> then it should have the correct size. I thought GraphicImport in
> writerfilter would only set position, margins and wrap related things as
> needed for Writer. Impress uses the same resulting shape from oox without
> any scaling.

I agree. My understanding is that if we calculate
translation/scaling/rotation only once, construct a transform matrix
from it and set that as an UNO property on the shape once, then there
are no surprises, that's what Impress does.

If we set position or size after the fact, that never plays nicely with
existing rotation.

I tried to fix position already in
6c4f737ec88a4f4dc5da8b2295ca5e7de2d4c24f, so that writerfilter/ informs
oox/ about what is the position and only oox/ sets it once. You could do
the same for size, and then writerfilter/ would not have to set the
size, I expect.

> Nevertheless I suggest to use the patch unless someone has concerns.

Sure, you don't have to do the above "set the size only once" in the
same change, it can be done in a follow-up change.

Regards,

Miklos
_______________________________________________
LibreOffice mailing list
LibreOffice@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/libreoffice