After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769505 - Set correct output scale on hot plug
Set correct output scale on hot plug
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2016-08-04 08:28 UTC by Jonas Ådahl
Modified: 2016-08-12 07:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
MetaMonitorManagerKms: Split up read_current() into logical chunks (28.08 KB, patch)
2016-08-04 08:28 UTC, Jonas Ådahl
committed Details | Review
monitor-manager: Always set the monitor info scale (1.04 KB, patch)
2016-08-04 08:28 UTC, Jonas Ådahl
committed Details | Review
MetaMonitorManagerKms: Set output scale when assigning crtc (1.16 KB, patch)
2016-08-04 08:28 UTC, Jonas Ådahl
none Details | Review
MetaMonitorManagerKms: Set output scale when assigning crtc (1.96 KB, patch)
2016-08-04 11:24 UTC, Jonas Ådahl
committed Details | Review

Description Jonas Ådahl 2016-08-04 08:28:06 UTC
We didn't calculate a style when reading the current configuration because the
connected output had not been assigned a crtc yet. The result was that external
HiDPI monitors would get the wrong scale on hot-plug.

While debugging the issue I decided to clean up the KMS configuration reading
code splitting up the mega function into chunks that were easier to reason
about.

The other two fixes the issue of this bug.
Comment 1 Jonas Ådahl 2016-08-04 08:28:10 UTC
Created attachment 332700 [details] [review]
MetaMonitorManagerKms: Split up read_current() into logical chunks

Instead of reading all the different state in one huge function, split
it up into logical chunks, making it easier to read.
Comment 2 Jonas Ådahl 2016-08-04 08:28:15 UTC
Created attachment 332701 [details] [review]
monitor-manager: Always set the monitor info scale
Comment 3 Jonas Ådahl 2016-08-04 08:28:20 UTC
Created attachment 332702 [details] [review]
MetaMonitorManagerKms: Set output scale when assigning crtc

The scale will have been set to 1 no matter what when initializing the
MetaOutput since it at the time didn't have an CRTC assigned to it.
Now, when we assign the CRTC to the output, we need to update the scale.
Comment 4 Jonas Ådahl 2016-08-04 11:24:43 UTC
Created attachment 332709 [details] [review]
MetaMonitorManagerKms: Set output scale when assigning crtc

The scale will have been set to 1 no matter what when initializing the
MetaOutput since it at the time didn't have an CRTC assigned to it.
Now, when we assign the CRTC to the output, we need to update the scale.

--

Moved the for loop under the crtc setup and fixed the get_output_scale() call.
My wip state had leaked over onto the patch.
Comment 5 Carlos Garnacho 2016-08-04 22:00:48 UTC
Comment on attachment 332700 [details] [review]
MetaMonitorManagerKms: Split up read_current() into logical chunks

Yay! I'd use the opportunity to kill some tabs leftovers in indentation, I see some esp. in the chunks that remained unchanged across the refactor.
Comment 6 Carlos Garnacho 2016-08-04 22:01:29 UTC
Comment on attachment 332701 [details] [review]
monitor-manager: Always set the monitor info scale

Makes sense
Comment 7 Carlos Garnacho 2016-08-04 22:02:15 UTC
Comment on attachment 332709 [details] [review]
MetaMonitorManagerKms: Set output scale when assigning crtc

Indeed
Comment 8 Jonas Ådahl 2016-08-12 07:28:23 UTC
(In reply to Carlos Garnacho from comment #5)
> Comment on attachment 332700 [details] [review] [review]
> MetaMonitorManagerKms: Split up read_current() into logical chunks
> 
> Yay! I'd use the opportunity to kill some tabs leftovers in indentation, I
> see some esp. in the chunks that remained unchanged across the refactor.

Tabs killed with fire.


Attachment 332700 [details] pushed as 6940169 - MetaMonitorManagerKms: Split up read_current() into logical chunks
Attachment 332701 [details] pushed as 1f657d2 - monitor-manager: Always set the monitor info scale
Attachment 332709 [details] pushed as 9f6f778 - MetaMonitorManagerKms: Set output scale when assigning crtc