Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for manually setting subpixel hinting on outputs. #3645

Merged
merged 1 commit into from
Mar 24, 2019

Conversation

ggreer
Copy link
Contributor

@ggreer ggreer commented Feb 11, 2019

Many laptop screens report unknown subpixel order. Allow users to manually set subpixel hinting to work around this.

Addresses #3163

TODO: update docs.

sway/config/output.c Outdated Show resolved Hide resolved
@RedSoxFan
Copy link
Member

Other points:

  • This needs to be reset on reload (see default_output_config)
  • This should probably be added to IPC_GET_OUTPUTS

@RedSoxFan
Copy link
Member

Also just as a note, since sway is currently in a feature freeze for the 1.0 tree, this will not make it into 1.0.

@ggreer ggreer force-pushed the output_subpixel branch 3 times, most recently from 6230211 to ade1fdf Compare February 11, 2019 02:47
@ggreer
Copy link
Contributor Author

ggreer commented Feb 11, 2019

ggreer@iron:~/code/sway% swaymsg -t get_outputs
Output eDP-1 'Unknown 0x06D0 0x00000000' (focused)
  Current mode: 2880x1920 @ 60.007000 Hz
  Position: 0,0
  Scale factor: 2.000000
  Subpixel hinting: rgb
  Transform: normal
  Workspace: 2
  Available modes:
    2880x1920 @ 60.007000 Hz

ggreer@zinc:~% swaymsg -t get_outputs
Output eDP-1 'Lenovo Group Limited 0x4005 0x00000000' (focused)
  Current mode: 1400x1050 @ 50.000000 Hz
  Position: 0,0
  Scale factor: 1.000000
  Subpixel hinting: rgb
  Transform: normal
  Workspace: 2
  Available modes:
    640x480 @ 59.939999 Hz
    800x600 @ 60.317001 Hz
    1024x768 @ 60.004002 Hz
    1280x1024 @ 60.020000 Hz
    1400x1050 @ 50.000000 Hz

I can also change the subpixel hinting at runtime with swaymsg output eDP-1 subpixel rgb. Some apps don't pick up the new hinting until they're restarted, but app title bars are updated immediately. The difference is extremely obvious on my laptop with a 1400x1050 screen.

@ggreer ggreer changed the title Add support for manually setting subpixel order on outputs. Add support for manually setting subpixel hinting on outputs. Feb 11, 2019
ggreer added a commit to ggreer/wlroots that referenced this pull request Feb 12, 2019
drmModeConnector.subpixel doesn't seem to detect subpixel order on many displays (especially laptops). Allow subpixel order to be manually set.

The corresponding PR for sway adds a subpixel output option: swaywm/sway#3645

Once both are merged, swaywm/sway#3163 will be fixed.
emersion pushed a commit to swaywm/wlroots that referenced this pull request Feb 13, 2019
drmModeConnector.subpixel doesn't seem to detect subpixel order on many displays (especially laptops). Allow subpixel order to be manually set.

The corresponding PR for sway adds a subpixel output option: swaywm/sway#3645

Once both are merged, swaywm/sway#3163 will be fixed.
@ggreer ggreer force-pushed the output_subpixel branch 2 times, most recently from 9db8fee to 78427f1 Compare February 15, 2019 02:46
@emersion emersion added the enhancement New feature or incremental improvement label Feb 15, 2019
@ggreer ggreer force-pushed the output_subpixel branch 2 times, most recently from df1fc6b to ebec8b1 Compare February 17, 2019 00:49
Copy link
Member

@RedSoxFan RedSoxFan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs a way for the value to reset subpixel on reload. Currently, it sets WL_OUTPUT_SUBPIXEL_UNKNOWN as part of the default output config, but apply_output_config does not call wlr_output_set_subpixel for WL_OUTPUT_SUBPIXEL_UNKNOWN. Ideally, it would reset to the value it had before any manual changes

@ggreer
Copy link
Contributor Author

ggreer commented Feb 26, 2019

Alright. I added the code to store the original subpixel value & reset it in apply_output_config(). In testing, I can run swaymsg output eDP-1 subpixel rgb and swaymsg -t get_outputs says hinting is rgb. Then I can reload my config and swaymsg -t get_outputs says unknown (which is the original value).

Copy link
Member

@RedSoxFan RedSoxFan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the most part this LGTM. I personally can't tell a difference, but I'll take your word for it that it changes. Can you just make the few small changes and squash the commits together?

include/sway/output.h Outdated Show resolved Hide resolved
sway/commands/output/subpixel.c Outdated Show resolved Hide resolved
sway/config/output.c Show resolved Hide resolved
sway/config/output.c Outdated Show resolved Hide resolved
@ggreer
Copy link
Contributor Author

ggreer commented Feb 26, 2019

Alright, changes made. Before merging, I want to test this again on a lower resolution display to make sure it still behaves correctly. I'm busy for the rest of the day, so it might be as late as tomorrow before I know for sure.

sway/config/output.c Outdated Show resolved Hide resolved
common/util.c Outdated Show resolved Hide resolved
common/util.c Outdated Show resolved Hide resolved
sway/config/output.c Show resolved Hide resolved
sway/ipc-server.c Outdated Show resolved Hide resolved
@ggreer ggreer force-pushed the output_subpixel branch 2 times, most recently from 985d4e9 to db939f7 Compare March 11, 2019 17:07
Many laptop screens report unknown subpixel order. Allow users to manually set subpixel hinting to work around this.

Addresses swaywm#3163
@emersion emersion merged commit 6e30468 into swaywm:master Mar 24, 2019
@emersion
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or incremental improvement
Development

Successfully merging this pull request may close these issues.

None yet

3 participants