Bug 145161 - Image compression dialog is misleadingly labelled: doesn't always "reduce image resolution" (comment 9)
Summary: Image compression dialog is misleadingly labelled: doesn't always "reduce ima...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
7.2.1.2 release
Hardware: All All
: medium normal
Assignee: Aditya Sahu
URL:
Whiteboard: target:25.2.0
Keywords: difficultyBeginner, easyHack, skillDesign, topicUI
Depends on:
Blocks: Image-Compression-Dialog
  Show dependency treegraph
 
Reported: 2021-10-15 19:57 UTC by Jérôme
Modified: 2024-11-02 16:44 UTC (History)
4 users (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 Jérôme 2021-10-15 19:57:02 UTC
The image compression form will save the new image with the target resolution even if the former image has a lower resolution that the target. This generally increases the image file size.
Comment 1 Timur 2021-10-16 05:41:56 UTC
You filed bug 145160 and bug 145161 probably without searching existing bugs first. Please do and see if a duplicate, maybe bug 132592.
Comment 2 Jérôme 2021-10-16 08:31:20 UTC
(In reply to Timur from comment #1)

This bug isn't a duplicate of bug 132592 since it focuses on a specific issue.

Bug 132592 is a set of different issues. Each issue of bug 132592 could be addressed by a different team and a specific timeline.
Comment 3 Jérôme 2021-10-17 16:41:36 UTC
See bug 77407 comment 9.
Comment 4 Buovjaga 2022-12-05 16:54:23 UTC
(In reply to Jérôme from comment #0)
> The image compression form will save the new image with the target
> resolution even if the former image has a lower resolution that the target.
> This generally increases the image file size.

UX team: are we taking away a legitimate use case, if this is restricted?
Comment 5 Heiko Tietze 2022-12-06 13:50:40 UTC
What do you expect from (any) tool if the source is 10ppi and you "compress" to 500ppi? Probably that increasing the size is not possible per maximum value.
Comment 6 Jérôme 2022-12-09 20:42:55 UTC
Version: 7.4.3.2 / LibreOffice Community
Build ID: 8d7af0b9f05ca3f6bf3593323f061d3291e2ce28
CPU threads: 4; OS: Linux 4.9; UI render: default; VCL: gtk3
Locale: fr-FR (fr_FR.utf8); UI: fr-FR
Calc: threaded

With this version, let's create a document (Writer or Impress) and insert an image which appears in ~300 dpi (jpeg 404 KiB in my example).

Then go to the contextual menu on the image and select "compress".

If you select the "reduce image resolution to:" checkbox and the 600 dpi resolution and press the "ok" button. Then you get a 600 dpi image (jpeg 1165 KiB in my example).

My point is that the words "reduce image resolution to" are wrong. If it were reduced, the original image would at least remain intact.
Comment 7 Jérôme 2022-12-10 13:28:13 UTC
> My point is that the words "reduce image resolution to" are wrong. If it
> were reduced, the original image would at least remain intact.

I propose that we keep the words "reduce image resolution to" and adjust the behaviour of this dialog : change the resolution of the image only if its resolution is reduced.

Example A :
1. original image is 250 dpi
2. select 300 dpi and click ok keep the original image intact

Exemple B :
1. original image is 250 dpi
2. select 150 dpi and click ok reduce the resolution of the image

Exemple C :
1. original image is 250 dpi
2. select 100 dpi and click ok reduce the resolution of the image


There is an additional feature (proposed in https://bugs.documentfoundation.org/show_bug.cgi?id=77407#c9 ) : make the example B keep the original image intact (since [original resolution] < 2 * [target resolution]).
Comment 8 Heiko Tietze 2022-12-12 09:16:24 UTC
(In reply to Jérôme from comment #7)
> Example A :
> 1. original image is 250 dpi
> 2. select 300 dpi and click ok keep the original image intact

Meaning you can configure something in the dialog and it has just no effect (nor feedback).

What I meant with maximum in comment 5 is to omit 600dpi in the dropdown list if the image has 300dpi. Could also be realized as slider with steps from min to max. But the point is: Do we really need to change this? Just to be consistent with the usual wording.
Comment 9 Heiko Tietze 2022-12-16 07:59:42 UTC
We discussed the topic in the design meeting.

While I commented before that limiting the resolution to the source dpi could be done it's not possible since we also allow to change width/height (or we set a maximum here too). And changing the image size affects the resolution (whether this is necessary is a different question).

So it's actually just the label that doesn't fit the current situation. And we propose to change it to "Change image resolution to:".

We also pondered quickly over the dialog title (and command name) "Compress Image" but better keep this familiar term.

Code pointer: svx/uiconfig/ui/compressgraphicdialog.ui

This ticket kind of duplicates bug 132657, up to QA.
Comment 10 Stéphane Guillou (stragu) 2022-12-16 18:24:27 UTC
(In reply to Heiko Tietze from comment #9)

> This ticket kind of duplicates bug 132657, up to QA.

Seeing the UX recommendations, let's keep this ticket as the relabelling easyHack. Changing summary accordingly.

Bug 132657 can continue the discussion around potential deeper changes, regarding size / resolution limits, syncing of values, etc.
Comment 11 Aditya Sahu 2024-10-21 03:53:20 UTC
I've given it a try here: https://gerrit.libreoffice.org/c/core/+/175260
Comment 12 Commit Notification 2024-10-21 14:40:46 UTC
Aditya Sahu committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/c49ec77db3c99b6b45f70bee18244f399bd78a68

tdf#145161: Change image compression dialog string

It will be available in 25.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 13 Buovjaga 2024-10-21 16:29:44 UTC
I pointed out some stuff in a review comment after the merge:

Should the checkbox name checkbox-reduce-resolution also have been changed? In addition to two changes in this .ui file, a change would also be required in svx/source/dialog/compressgraphicdialog.cxx, if so.

A further change is needed in any case: filter/uiconfig/ui/pdfgeneralpage.ui has

<property name="label" translatable="yes" context="pdfgeneralpage|reduceresolution">Reduce ima_ge resolution to:</property>

Also, helpcontent2 mentions these strings in source/text/shared/01/image_compression.xhp and source/text/shared/01/ref_pdf_export_general.xhp
Comment 14 Heiko Tietze 2024-10-22 07:30:13 UTC
(In reply to Buovjaga from comment #13)
> Should the checkbox name checkbox-reduce-resolution also have been changed?
+1
> In addition to two changes in this .ui file, a change would also be required
> in svx/source/dialog/compressgraphicdialog.cxx, if so.
Why?
Comment 15 Buovjaga 2024-10-22 08:07:08 UTC
(In reply to Heiko Tietze from comment #14)
> > In addition to two changes in this .ui file, a change would also be required
> > in svx/source/dialog/compressgraphicdialog.cxx, if so.
> Why?

$ git grep -in reduce svx/source/dialog/compressgraphicdialog.cxx
svx/source/dialog/compressgraphicdialog.cxx:48:         bool ReduceResolutionCB = true;
svx/source/dialog/compressgraphicdialog.cxx:97:     m_xReduceResolutionCB->set_active( memp.ReduceResolutionCB );
svx/source/dialog/compressgraphicdialog.cxx:98:     if (memp.ReduceResolutionCB && (memp.MFNewWidth > 1))
svx/source/dialog/compressgraphicdialog.cxx:100:    if (memp.ReduceResolutionCB && (memp.MFNewHeight > 1))
svx/source/dialog/compressgraphicdialog.cxx:112:    UpdateSensitivity(m_xReduceResolutionCB->get_active());
svx/source/dialog/compressgraphicdialog.cxx:128:    m_xReduceResolutionCB = m_xBuilder->weld_check_button(u"checkbox-reduce-resolution"_ustr);
svx/source/dialog/compressgraphicdialog.cxx:149:    m_xReduceResolutionCB->connect_toggled( LINK( this, CompressGraphicsDialog, ToggleReduceResolutionRB ) );
svx/source/dialog/compressgraphicdialog.cxx:157:    m_xReduceResolutionCB->set_active(true);
svx/source/dialog/compressgraphicdialog.cxx:266:    if ( m_xReduceResolutionCB->get_active() )
svx/source/dialog/compressgraphicdialog.cxx:290:    memp.ReduceResolutionCB = m_xReduceResolutionCB->get_active();
svx/source/dialog/compressgraphicdialog.cxx:364:IMPL_LINK_NOARG( CompressGraphicsDialog, ToggleReduceResolutionRB, weld::Toggleable&, void )
svx/source/dialog/compressgraphicdialog.cxx:366:    UpdateSensitivity(m_xReduceResolutionCB->get_active());
svx/source/dialog/compressgraphicdialog.cxx:401:    if ( m_xReduceResolutionCB->get_active() )
svx/source/dialog/compressgraphicdialog.cxx:444:        if ( m_xReduceResolutionCB->get_active() )
Comment 16 Heiko Tietze 2024-10-22 09:03:54 UTC
(In reply to Buovjaga from comment #15)
> $ git grep -in reduce svx/source/dialog/compressgraphicdialog.cxx

memp is a variable to store the dialog's configuration during the session.

The variable names m_xReduce* don't matter and renaming it to m_xChange* has no effect (besides a more clear relation to the UI element).
Comment 17 Buovjaga 2024-10-22 09:34:13 UTC
(In reply to Buovjaga from comment #15)
> svx/source/dialog/compressgraphicdialog.cxx:128:    m_xReduceResolutionCB =
> m_xBuilder->weld_check_button(u"checkbox-reduce-resolution"_ustr);

Heiko, please pay attention to this.
Comment 18 Aditya Sahu 2024-10-23 02:27:06 UTC
Yeah, I believe that ought to be changed (Comment #17)
Comment 19 Jérôme 2024-10-23 17:02:00 UTC
Instead of changing the label, we could perhaps modify the behavior : change the resolution only if the target resolution is lower than the previous resolution.
Comment 20 Commit Notification 2024-11-02 08:59:45 UTC
Aditya Sahu committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/412a775d0ab90cd963216c40e9ae1189179809ba

tdf#145161: Change pdf and image compression dialog string

It will be available in 25.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 21 Commit Notification 2024-11-02 16:44:38 UTC
Ilmari Lauhakangas committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/help/commit/40bbfa8f4fb070b66481068b952755d04854db2c

tdf#145161 Update Help per renamed label, Change image resolution