Description: Freeze changing height to 75% relative Steps to Reproduce: 1. Open the attached file 2. Select the yellow smiley on top of the page 3. Press F4 4. Position and Size tab -> Size: Height to 75% 5. Press OK Actual Results: Freeze Expected Results: No freeze Reproducible: Always User Profile Reset: No Additional Info: Found in Version: 7.5.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 9cd0f4c2d25462feba0ffcbd906c199273821243 CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win Locale: nl-NL (nl_NL); UI: en-US Calc: CL threaded still Ok with Version: 7.1.8.0.0+ (x64) / LibreOffice Community Build ID: a94b58277c7aeaa83ce14347cd0b8f7137969d03 CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win Locale: nl-NL (nl_NL); UI: en-US Calc: CL
Created attachment 183347 [details] Example file
I confirm it with Version: 7.5.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 55cd20e6228a06836285c14ca6726adb1bb4ffcb CPU threads: 4; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win Locale: en-US (de_DE); UI: en-GB Calc: CL threaded and with Version: 7.2.7.2 (x64) / LibreOffice Community Build ID: 8d71d29d553c0f7dcbfa38fbfda25ee34cce99a2 CPU threads: 4; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win Locale: de-DE (de_DE); UI: de-DE Calc: CL only freezes, if keep ratio is checked O.K. in Version: 7.0.6.2 (x64) Build ID: 144abb84a525d8e30c9dbbefa69cbbf2d8d4ae3b CPU threads: 4; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win Locale: de-DE (de_DE); UI: en-GB Calc: CL
And.... what commit was this bisected to?
bibisected in version: Version: 7.2.0.0.alpha1+ / LibreOffice Community Build ID: 556243467a0ac3f647de75bf3fb6c9f3b72466a4 CPU threads: 3; OS: Linux 5.10; UI render: default; VCL: gtk3 Locale: en-IN (en_IN); UI: en-US Calc: threaded repo: https://bibisect.libreoffice.org/linux-64-7.2.git-bundle Commit info: commit caeb079b2a172bc085a32c3165aa171919dcab36 Author: Jenkins Build User <tdf@pollux.tdf> Date: Fri Jun 4 14:34:25 2021 +0200 source 556243467a0ac3f647de75bf3fb6c9f3b72466a4 source 556243467a0ac3f647de75bf3fb6c9f3b72466a4 instdir/program/libvcllo.so | Bin 16254488 -> 16254488 bytes instdir/program/setuprc | 2 +- instdir/program/versionrc | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) Adding CC: to Noel Grandin author Noel Grandin <noel.grandin@collabora.co.uk> Wed Jun 02 13:06:54 2021 +0200 committer Noel Grandin <noel.grandin@collabora.co.uk> Wed Jun 02 20:30:59 2021 +0200 attached: bibisecting done in command line
Created attachment 185882 [details] bibisecting done in command line
Thanks for the bibisect. Hmmm. I tried reverting that commit on master, and it did not fix the problem. Also, that commit does not look like the kind of thing that should cause a problem (like this). This is a layout loop bug (as opposed to so kind of locking issue). Which means that it might not happen 100% of the time, which is why I think the last few steps of the bibisect might have led to the wrong answer. Could you please try the last 2 or 3 steps of the bibisect again?
Created attachment 185907 [details] repeated bibisecting you are welcome! i repeated the entire bibisect as i was not sure how to try the last 2 or 3 steps. it resulted in the same 'first bad commit'. bibisecting observations: a) general - oldest - bug not repro - during image resize to 75% (with 'Keep Ratio' checked), app freeze was not seen b) general - master - bug repro - app freeze was seen c) between steps 10 and 9 - the app did not open - hence it was marked as the first 'bad' d) between steps 6 and 5 - file recovery popup appeared (app freeze was not seen) - it was marked 'good' since app freeze was not seen e) between steps 3 and 2 - the app did not open for the second time - hence marked it as the second 'bad' f) general - from steps 12 to 0 - bug not repro - app freeze was never seen
Hmmm, so this bug went wrong in stages. Bibisect was probably thrown off because the builds that didn't start. Compiling from source, shows that the layout/rendering regressed in commit 02c435082058ecf7f9d4d73cb47d31d0218dc10d (HEAD) Author: Miklos Vajna <vmiklos@collabora.com> Date: Mon Jun 7 18:03:33 2021 +0200 sw keep aspect ratio: add filter for this setting Still figuring out when the freeze happened.
Bisecting from source shows that the freeze started with commit dd5039ceea319ebfc72a3e032753b22538add12e Author: Heiko Tietze <tietze.heiko@gmail.com> Date: Tue Aug 24 11:14:46 2021 +0200 Resolves tdf#143633 - Image size wrong after disabling relative image width
Note that possibly the bug was there earlier, just once there is filter/UI for this, it's easier to trigger it. Perhaps previously a macro was needed to hit the problem.
Discussion from IRC: <vmiklos> noelgrandin: sure, the bugdoc contains a KeepRatio setting and the commit in question adds the import for it :) so the feature is simply lost before, which means it's harder to see the breakage <noelgrandin> aha! <vmiklos> it's definitely a bug, i am just not sure it's a recent problem <noelgrandin> hmm, probably if KeepRation==true, then the UI elements that allow adjusting the relative width/height should be disabled? <vmiklos> noelgrandin: KeepRatio assumes that one of the dimensions have a value (percentage or fixed). so "keep ratio" for both width and height is broken, but width=50% + keep ratio for height is OK <vmiklos> since that 50% will be 50% of something, e.g. page width without margins or so <noelgrandin> vmiklos, keep ratio is a single setting, not one for each of width height. So I still don't see how having KeepRatio=true __and__ having both of Width and Height relative to something is legal - IMO that will obviously lead to constraints that cannot be simultaneously satisfied <vmiklos> noelgrandin: yes, you're right. if KeepRatio is true, then you should only set either width or height and it's ideal if the UI enforces this So basically, this document is broken - on import, we should either (a) set KeepRatio to false or (b) Set one of the Width or Height to non-relative. And we should enforce this in the UI, it should not be possible to set Width and Height to relative, __and__ set KeepRatio to true.
(In reply to Noel Grandin from comment #9) > Bisecting from source shows that the freeze started with > commit dd5039ceea319ebfc72a3e032753b22538add12e Which is a follow-up to eb807e785b6acda1529b2f58ada2c1e84b38f7f5. IIRC, I just removed the nested GtkGrids to solve bug 140511, no functional change. See also bug 143633 about the follow-up (forgot the GtkAdjustments). (In reply to Noel Grandin from comment #11) > And we should enforce this in the UI, it should not be possible to set Width > and Height to relative, __and__ set KeepRatio to true. Don't get it why absolute/relative should conflict with "keep ratio". Sounds like band-aid.
(In reply to Heiko Tietze from comment #12) > > Don't get it why absolute/relative should conflict with "keep ratio". Sounds > like band-aid. They both imply constraints on the width/height. Sometimes it is not possible to satisfy both those constraints at the same time.
Keep ratio means to change one number and apply the same to the corresponding field. Whether you change 10cm / 8cm (absolute) to 5 = 4 or 100% or 60% / 40% (relative to paragraph) to 30 = 20 (which is how it works now). Cannot wrap my mind around mixed settings with 10cm absolute width by 50% relative height; is keeping the ratio still 5cm/25%?
I can't reproduce the loop in current master since: https://git.libreoffice.org/core/commit/c8cb7a658b33e296024044f3377378644a9a97ef tdf#144494 sw: fix layout loop *** This bug has been marked as a duplicate of bug 144494 ***