Bug 427803 - Vertical wallpaper of 5.20 is rotated instead of cropped while maintaining its orientation
Summary: Vertical wallpaper of 5.20 is rotated instead of cropped while maintaining it...
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Image Wallpaper (show other bugs)
Version: 5.20.0
Platform: Other Linux
: NOR normal
Target Milestone: 1.0
Assignee: Marco Martin
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2020-10-16 13:53 UTC by Claudius Ellsel
Modified: 2021-01-14 21:26 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Screenshot showing the setup (scaled down because of the file size limits here) (2.81 MB, image/png)
2020-10-16 13:58 UTC, Claudius Ellsel
Details
Old working setup (5.19) (3.36 MB, image/png)
2020-10-16 14:00 UTC, Claudius Ellsel
Details
Correctly vertically cropped example (1.05 MB, image/jpeg)
2021-01-14 17:04 UTC, Claudius Ellsel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Claudius Ellsel 2020-10-16 13:53:44 UTC
SUMMARY
-> See title

STEPS TO REPRODUCE
1. See monitor arrangement in the screenshot, might also happen on just a single vertical screen.

OBSERVED RESULT
The wallpaper is not rotated correctly on the left vertical monitor

EXPECTED RESULT
Correctly rotated wallpaper

SOFTWARE/OS VERSIONS
Operating System: Manjaro Linux
KDE Plasma Version: 5.20.0
KDE Frameworks Version: 5.75.0
Qt Version: 5.15.1
Kernel Version: 5.8.14-1-MANJARO
OS Type: 64-bit
Processors: 4 × Intel® Xeon® CPU E3-1225 v3 @ 3.20GHz
Memory: 11.6 GiB of RAM
Graphics Processor: Mesa DRI Intel® HD Graphics P4600/P4700

ADDITIONAL INFORMATION
Possibly a regression, worked fine with the previous wallpaper. Not tested whether altering the screen in the display settings helps to get it corrected.
Comment 1 Claudius Ellsel 2020-10-16 13:58:56 UTC
Created attachment 132410 [details]
Screenshot showing the setup (scaled down because of the file size limits here)
Comment 2 Claudius Ellsel 2020-10-16 14:00:02 UTC
Created attachment 132412 [details]
Old working setup (5.19)
Comment 3 Nate Graham 2021-01-07 15:50:18 UTC
If you think this might be wallpaper-related, does it happen with the new Milky Way wallpaper for Plasma 5.21 that's current;y in git master?
Comment 4 Claudius Ellsel 2021-01-08 16:15:48 UTC
I'll try to test soon on a KDE Neon unstable VM!
Comment 5 Claudius Ellsel 2021-01-13 15:10:49 UTC
I had a short test with my Neon unstable VM. Unfortunately I could not even reproduce it there with the 5.20 wallpaper. After updating, it also worked with the new one. While the problem happens for me on Manjaro and Tumbleweed when I select the 5.20 wallpaper.

So ideally I'd test with a compiled session, but don't have the energy currently.

Can you maybe link the MR where you introduced the new wallpaper, so I can have a look what has been changed?
Comment 7 Claudius Ellsel 2021-01-13 20:52:41 UTC
Thanks! With those I can confirm that it is probably all about the files. The vertical ones were rotated instead of cropped for 5.20. An example is https://invent.kde.org/plasma/breeze/-/commit/51840ad0686cf1182ce0ee1810864a5725412e1b#49f36dfe49f46c6ebc8e47db50a9b5baaf80ea32. You can see the difference to the new one that is cropped instead of rotated.

So, I guess in this stage it is a bit pointless to fix it for 5.20 (not sure whether there even is another point release this could go into)?
Comment 8 Nate Graham 2021-01-13 21:15:00 UTC
Ahhhhhhhhhhhh I understand the bug now. The rotated wallpaper in Plasma 5.20 didn't actually change its dimensions, but rather had a rotation tag attached, as is typical for rotated images. The algorithm  to determine the best-fitting image doesn't take that into account.
Comment 9 Claudius Ellsel 2021-01-13 21:20:26 UTC
Are you sure? See my comment there (I guess that is the algorithm you are referring to?): https://invent.kde.org/plasma/breeze/-/commit/c6cb949de22b092359dccf859a950d1336467a3a#note_167338. To me it just seems like the base image for vertical is not correct. Or is there a different algorithm/other problem?
Comment 10 Nate Graham 2021-01-13 22:22:03 UTC
Different algorithm.
Comment 11 Claudius Ellsel 2021-01-14 12:56:10 UTC
I am still not convinced this is the root of the problem.
To me the simpler reason that the vertical images for 5.20 were "wrong" is the more likely explanation currently. I mean, one can clearly see they are "wrong" already on the commit on GitLab, so I assume the wallpaper size chooser algorithm (assuming you are talking about an algorithm in Plasma desktop determining which wallpaper file to use for a monitor) worked fine and selected a correct vertical one, but the content of the image was "wrong" already.
Comment 12 Claudius Ellsel 2021-01-14 15:26:47 UTC
One more data point (which might only add confusion to this, but anyway): The orientation is correct on the lockscreen (not SDDM but the other one - Shortcut Meta + L).
Comment 13 Nate Graham 2021-01-14 16:50:28 UTC
> To me the simpler reason that the vertical images for 5.20 were "wrong"
And in what way were they wrong?
Comment 14 Claudius Ellsel 2021-01-14 17:00:11 UTC
They are rotated to the vertical aspect ration instead of cropped (which would retain their orientation). Look at https://invent.kde.org/plasma/breeze/-/blob/6c72bd648e6e97f52fb255e5c1d69d1e143a89ec/wallpapers/Next/contents/images/vertical_base_size.jpg - The content is rotated by 90 degrees clockwise there.

The content should not be rotated to get a vertical aspect ratio (you can compare that to the new one for example, where it is correct: https://invent.kde.org/plasma/breeze/-/blob/51840ad0686cf1182ce0ee1810864a5725412e1b/wallpapers/Next/contents/images/vertical_base_size.png, alternatively the previous one, also correctly oriented: https://invent.kde.org/plasma/breeze/-/blob/9873c57ea38348636f82665d3f933948a94ba06e/wallpapers/Next/contents/images/720x1440.jpg).
Comment 15 Claudius Ellsel 2021-01-14 17:04:36 UTC
Created attachment 134856 [details]
Correctly vertically cropped example

To be completely clear and avoid misunderstanding: This attachement shows a quick example of how it *should* look like instead.
Comment 16 Nate Graham 2021-01-14 19:46:32 UTC
So in other words, the root cause of the bug the EXIF rotation tag in the rotated image, right?
Comment 17 Claudius Ellsel 2021-01-14 19:57:39 UTC
No, if the image would have been rotated by moving pixels around instead of adding a rotation tag the problem would be exactly the same. It should not have been rotated at all, but cropped instead while maintaining orientation of the content.
Comment 18 Nate Graham 2021-01-14 21:12:43 UTC
So you're objecting to the aesthetic choices of how the landscape version of the Shell wallpaper was modified for its portrait version?
Comment 19 Claudius Ellsel 2021-01-14 21:18:37 UTC
Yes, if you consider rotating it by 90 degrees a concious cosmetic choice. I don't think that was on purpose.

Best read the description of this issue again and look at the first attachement. That sums it up pretty well.
Comment 20 Nate Graham 2021-01-14 21:20:38 UTC
I see.

In that case there is no point to this discussion because we have already changed the wallpaper in Plasma 5.21 to one that you do not find objectionable, and there are no further Plasma 5.20 releases planned with which we would have the opportunity to make any changes.
Comment 21 Nate Graham 2021-01-14 21:21:22 UTC
FWIW, rotating the Shell wallpaper for the portrait version was indeed intentional.
Comment 22 Claudius Ellsel 2021-01-14 21:24:59 UTC
(In reply to Nate Graham from comment #20)
> I see.
> 
> In that case there is no point to this discussion because we have already
> changed the wallpaper in Plasma 5.21 to one that you do not find
> objectionable, and there are no further Plasma 5.20 releases planned with
> which we would have the opportunity to make any changes.

Alright, makes sense. Pretty much what I guessed in comment #7. Sorry that there apparently was misunderstanding afterwards.

(In reply to Nate Graham from comment #21)
> FWIW, rotating the Shell wallpaper for the portrait version was indeed
> intentional.

Lol, did not anticipate that :) Maybe worth noting that such things might confuse *some* users of multimonitor setups ;)