Hi Miklos, hi all,
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
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.
Nevertheless I suggest to use the patch unless someone has concerns.
Kind regards
Regina
Miklos Vajna schrieb am 16.04.2021 um 09:31:
Hi Regina,
On Fri, Apr 16, 2021 at 12:42:28AM +0200, Regina Henschel <rb.henschel@xxxxxxxxxxx> wrote:
Now I stuck, because some unit tests fail with small differences. But before
I put a lot of time into it, to find whether the values in the unit tests
are wrong or whether and where my patch is incorrect, I would like, if you
have a look at the patch in general.
The approach looks reasonable to me in general. As I understand it, the
LO group shape is a simple container of shapes (with a name), while the
MSO group shape can have its own rotation, scaling, etc -- so their
document model can be more complex. Based on this, it's expected that
you may need the "original" rotation of the group shape in
writerfilter/.
Regarding the unit tests: I try to always document how a test failed
without the original fix. So in case you see that e.g. now the test
fails because of a 98 vs 99 difference and the comment says the old
value was 42, then this may suggest that only the test was poor and it
can be adapted.
You can always manually compare the import result of a particular test
document with and without your fix: if you see no differences, then that
also suggests that you are OK to change the test.
Otherwise it's likely that the test caught some badness and you need to
improve your patch.
Hope that helps,
Miklos
_______________________________________________
LibreOffice mailing list
LibreOffice@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/libreoffice
