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

windowing/gbm: add option to limit gui size #16063

Merged
merged 1 commit into from Apr 12, 2020

Conversation

Kwiboo
Copy link
Member

@Kwiboo Kwiboo commented May 4, 2019

Description

This PR adds a new video setting Limit GUI Size for GBM platform that can be used on devices / SoCs with weaker GPUs in order to improve user experience by limiting the gui plane size used.

This setting is <visible>false</visible> by default and is only intended to be set visible or forced in appliance.xml for devices / SoCs with weaker GPUs.

Motivation and Context

Allows for video to render in 4K using direct-to-plane rendering and having a 720/1080p gui plane for osd / subtitles.

How Has This Been Tested?

This has been included in LibreELEC Rockchip, Allwinner and mainline Amlogic builds for a long time.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@Kwiboo Kwiboo mentioned this pull request May 4, 2019
@lrusak lrusak added this to the Matrix 19.0-alpha 1 milestone May 7, 2019
@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label May 25, 2019
@jenkins4kodi jenkins4kodi removed the Rebase needed PR that does not apply/merge cleanly to current base branch label May 30, 2019
@Kwiboo Kwiboo marked this pull request as ready for review July 1, 2019 20:30
@Kwiboo
Copy link
Member Author

Kwiboo commented Jul 1, 2019

This is now used on Allwinner, Amlogic, Rockchip and RPi4 in LibreELEC. Should be ready for merging unless we want to add support for using 720p as gui size when a 1080p resolution is used, current limits only kicks in when resolution is above 1080p.

@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Jul 10, 2019
@jenkins4kodi jenkins4kodi removed the Rebase needed PR that does not apply/merge cleanly to current base branch label Jul 10, 2019
diogomsantos added a commit to PIPplware/xbmc that referenced this pull request Oct 16, 2019
diogomsantos added a commit to PIPplware/xbmc that referenced this pull request Oct 16, 2019
@Kwiboo
Copy link
Member Author

Kwiboo commented Oct 17, 2019

I plan on merging this unless anyone have any objection, ping @lrusak

Windows UWP build seems to have failed due to a Python3 issue.

@Kwiboo
Copy link
Member Author

Kwiboo commented Oct 17, 2019

Yes I am aware, I added a new option and labels because although the videoscreen.limitgui is used for similar purposes the limitations is not applied in the same way.

To my knowledge the videoscreen.limitgui is a hard limit that always applies to GUI regardless of what screen resolution is used.
E.g. with the 720p option selected the GUI would be limited to 720p for any screen resolution above 720p.

This new option was originally intended to only apply for screen resolutions above 1080p (1440p, 2160p etc).
E.g. with the 720p option selected and a screen resolution of 2160p, the GUI would be limited to 720p, and for a 1080p screen resolution the GUI would not be limited and would instead use a 1080p buffer.

The reasoning behind this is that for some embedded devices (RK3328) there is only enough memory bandwidth to display a 2160p video buffer and a 720p gui buffer at the same time for 2160p screen resolutions with refresh rates above 30hz. Using a 2160p video buffer and a 1080p gui buffer may work with 2160p@30hz or less hz. Other devices (RK3399) may have enough memory bandwidth to display 2x 2160p buffers but are instead limited by GPU power and can only cope with GUI at 2160p up to 30fps.

Any suggestions on how to merge both these two options into one is welcomed!

@ksooo
Copy link
Member

ksooo commented Oct 18, 2019

I think you should at least reuse the strings from strings.po.

nsenica pushed a commit to PIPplware/xbmc that referenced this pull request Nov 22, 2019
0p1pp1 pushed a commit to 0p1pp1/xbmc that referenced this pull request Nov 27, 2019
nsenica pushed a commit to PIPplware/xbmc that referenced this pull request Dec 8, 2019
@phunkyfish
Copy link
Contributor

@Kwiboo do you want to progress this, close it or put it on the backburner?

@phunkyfish phunkyfish added the PR Cleanup: Ask Author PR appears to still be relevant and mergeable. Comment to author to check on status. label Mar 9, 2020
@lrusak
Copy link
Contributor

lrusak commented Apr 5, 2020

Updated PR here: https://github.com/jernejsk/xbmc/tree/gbm-limitguisize
It seems I have no rights to push to kwiboo's branch.

@jernejsk I tried also but couldn't get it to work. I think it's because his xbmc repo is a fork of plexinc/plex-home-theater-public and not xbmc/xbmc

@Kwiboo
Copy link
Member Author

Kwiboo commented Apr 5, 2020

Thanks @jernejsk, I have pushed your branch now

@lrusak lrusak requested a review from ksooo April 6, 2020 00:23
@lrusak
Copy link
Contributor

lrusak commented Apr 6, 2020

added @ksooo again to check if the changes make him happy 😄

Copy link
Contributor

@lrusak lrusak left a comment

Choose a reason for hiding this comment

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

good to go other than that small string change

@jernejsk
Copy link
Contributor

Any objection against merging this in few days?

@lrusak lrusak merged commit ed7f1c2 into xbmc:master Apr 12, 2020
@Kwiboo Kwiboo deleted the gbm-limitguisize branch April 13, 2020 10:54
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 14, 2020
windowing/gbm: add option to limit gui size
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 16, 2020
windowing/gbm: add option to limit gui size
Maven85 pushed a commit to Maven85/kodi that referenced this pull request May 5, 2020
windowing/gbm: add option to limit gui size
nsenica pushed a commit to PIPplware/xbmc that referenced this pull request May 6, 2020
nsenica pushed a commit to PIPplware/xbmc that referenced this pull request May 28, 2020
nsenica pushed a commit to PIPplware/xbmc that referenced this pull request May 28, 2020
nsenica pushed a commit to PIPplware/xbmc that referenced this pull request Jun 12, 2020
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 3, 2020
windowing/gbm: add option to limit gui size
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
windowing/gbm: add option to limit gui size
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
windowing/gbm: add option to limit gui size
diogomsantos added a commit to PIPplware/xbmc that referenced this pull request Aug 4, 2020
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
windowing/gbm: add option to limit gui size
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
windowing/gbm: add option to limit gui size
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
windowing/gbm: add option to limit gui size
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
windowing/gbm: add option to limit gui size
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 7, 2020
windowing/gbm: add option to limit gui size
nsenica pushed a commit to PIPplware/xbmc that referenced this pull request Oct 27, 2020
nsenica pushed a commit to PIPplware/xbmc that referenced this pull request Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: GLES rendering Component: GUI rendering Platform: Linux PR Cleanup: Ask Author PR appears to still be relevant and mergeable. Comment to author to check on status. Type: Feature non-breaking change which adds functionality v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants