Bug 141761 - The Display of FormControls in EditMode or PrintPreview may vanish unexpectedly
Summary: The Display of FormControls in EditMode or PrintPreview may vanish unexpectedly
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Armin Le Grand (allotropia)
URL:
Whiteboard: target:7.2.0 target:7.1.4
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-19 15:20 UTC by Armin Le Grand (allotropia)
Modified: 2021-06-09 11:22 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Armin Le Grand (allotropia) 2021-04-19 15:20:58 UTC
While fixing another Task I stumbled over a not reliably reproducible, but possible error: The Display/Paint of FormControls may vanish. This can happen
- when FormControls are in EditMode (else they are Windows)
- in the Preview of the Print-Dialog (Print-Preview maybe ,too)
- when zooming in/out in that mode
Unfortunately I cannot provide the Test-Document, but a simple, self-produced can be used, New Writer, some tex, add a CheckBox FormControl, go in EditMode, Zoom in/out. As said interactive reproduction is not guaranteed.

But from the code there is a simple possibility (needs edit/compile, sorry):
- go to Button::ImplDrawRadioCheck in vcl/source/control/button.cxx
- after 'aSize.AdjustWidth( -(rImageSize.Width() + nImageSep) );'
- add code to set aSize.Width() and/or Height to Zero
- e.g.: aSize.setWidth(0)
- copmpile, let run, open testdoc, look at PrintPreview
-> FormControl disappears.

Background:
The code mentioned above in Button::ImplDrawRadioCheck uses

    aSize.AdjustWidth( -(rImageSize.Width() + nImageSep) );

to make the optical relation (historical reasons?) nicer/better_fitting to the following text. Problem is that this expression *can* by coincidence get zero, thus creating the case described above. Since

   const tools::Long nImageSep = GetDrawPixel( pDev, ImplGetImageToTextDistance() );

is calculated using the current Zoom (CalcZoom() called) and thus being influenced by MapMode &/| DPI of the system.
In my error-case I saw values of 25, 30 for nImageSep, adding to zero with aSize being 25 before or even to a Width of '-5' with a Size of 30 (strange itself - negative width ... that's why I added B2DRange/B2Iange based on min/max so this cannot happen by definition).

I started debugging and this goes down to ReferenceDeviceTextLayout::DrawText using m_rTargetDevice.PixelToLogic to transform the values. Unfortunately, OutputDevice::PixelToLogic tests

    if ( !mbMap || rDeviceRect.IsEmpty() )
        return rDeviceRect;

and thus does not transform the Rectangle which compromizes it's position. As can be seen a Rectangle may be used to transport a Position, so the Position *should* be transformed...
Comment 1 Armin Le Grand (allotropia) 2021-04-19 15:22:02 UTC
Grepping, debugging, working on a fix && testing it carefully...
Comment 2 Armin Le Grand (allotropia) 2021-04-20 08:28:06 UTC
1st try failed, so *have* to correct the seven transformations of Rectangle in vcl/source/outdev/map.cxx handish and oriented on if IsEmpty() is already used. Also, Rectangle::RECT_EMPTY is not accessible from outside, and this is good that way, so I'll keep that. There *is* a 1st thought to use it as value and hand over in the Rectangle 4-Value constructor, but better not...
This will work and be safe due to using info about Rectangle already being interpreted as non-pixel-oriented (aka being IsEmpty() or Right||Bottom==RECT_EMPTY) and keeping that state.

Adding comment:
Even if rLogicRect.IsEmpty(), transform of the Position contained in the Rectangle is necessary. Due to Rectangle::Right() returning Left() when IsEmpty(), the code *could* stay unchanged (same for Bottom), but:
The Rectangle constructor used with the four tools::Long values does not
check for IsEmpty(), so to keep that state correct there are two possibilities:
(1) Add a test to the Rectangle constructor in question
(2) Do it handish here
I have tried (1) first, but test Test::test_rectangle() claims that for
  tools::Rectangle aRect(1, 1, 1, 1);
    tools::Long(1) == aRect.GetWidth()
    tools::Long(0) == aRect.getWidth()
  (remember: this means Left == Right == 1 -> GetWidth => 1, getWidth == 0)
so indeed tthe 1's have to go uncommened/unchecked into the data body
of rectangle. Switching to (2) *is* needed, doing so

To understand (better: accept - or live with) Rectangle in it's current form you really have to know where it comes from - 25~30 years ago, no AA, Controls and UI painted Pixel-oriented. For that reason a Rectangle with Left == Right was defined to be Width => 1 due to *covering* one pixel - in discrete units...
To clean this up, B2DRange/B2IRange and a clearly Pixel-orineted version of Rectangle would've to be used - but usages can nearly not be sorted out anymore. People not being aware of the discrete Pixel-oriented nature of Rectangle used it for logic coordinates - and cannot be blamed, how could you know...?
Kudos to already trying to make it more safe in it's definitions, thjat's the right direction to go!
Comment 3 Armin Le Grand (allotropia) 2021-04-20 15:26:16 UTC
2nd version went through gerrit (https://gerrit.libreoffice.org/c/core/+/114304). This does not automatically mean that all will work. Applying aSize.setWidth(0) as in Description -> still happens, so need to debug after the transformation...
Comment 4 Armin Le Grand (allotropia) 2021-04-22 07:40:14 UTC
I have a minimal impact solution ready (after deep-dive and checking through the Control::Paint stack, all Pixel-oriented, grown stuff and some experimenting). Doing some more tests, preparing push to gerrit...
Comment 5 Armin Le Grand (allotropia) 2021-04-22 08:46:50 UTC
New version at gerrit, checking on 2nd system...
Comment 6 Commit Notification 2021-04-22 14:56:24 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/35e4a45260f128f353d25e2a2f2b800e6bd11d61

tdf#141761 Avoid vanishing FormControls

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Commit Notification 2021-04-26 17:33:05 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

https://git.libreoffice.org/core/commit/55a18fcee6ba152e9db154cc925d2f578f31a9ae

tdf#141761 Avoid vanishing FormControls

It will be available in 7.1.4.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Armin Le Grand (allotropia) 2021-04-27 09:01:58 UTC
Print is OK and FormControls are somewhat visible in Preview, but Quality is bad.
Found out why the print preview looks bad:

(1) the orig page metafile gets scaled using Metafile::Scale() with 0.1/0.1 values

(2) a pre-rendered bitmap is created

(3) that one is scaled ca. 0.5/0.5 (highest quality)

Number (1) is the problem, the bitmap in (2) already has lines of the FormControls missing. Probably because a ClipRegion and the lines making up the FormControl start to overlap by that scale reducing by factor 10.

-> changing bm scale quality changes nothing here
-> avoiding (1) would either mean (a) to always paint the metafile instead of a prep bitmap, also with a scaling, so not sure if that would save the lines - or (b) to change to primitives and use a transformation instead of a scale (but PrintDialog is in vcl so primitives and PrimitiveRendrers are not usable) - sigh

-> no good fast solution available....
Comment 9 Armin Le Grand (allotropia) 2021-04-27 09:02:48 UTC
Found a acceptable 'workaround' as long as no redesign using better technics is available. Preparing...
Comment 10 Commit Notification 2021-04-27 14:03:08 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/4722ad2cf3f2b91c217e3548f811f2972f2aa60c

tdf#141761 Enhance PrintDialog Preview for FormControls

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2021-05-01 22:58:43 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

https://git.libreoffice.org/core/commit/34ed5f0b28942948210a39bd92e57a30aa6385f2

tdf#141761 Enhance PrintDialog Preview for FormControls

It will be available in 7.1.4.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.