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

Rewrite GUI/layout code for 1080p #74

Closed
3 of 11 tasks
cewert opened this issue Oct 5, 2019 · 9 comments
Closed
3 of 11 tasks

Rewrite GUI/layout code for 1080p #74

cewert opened this issue Oct 5, 2019 · 9 comments
Labels
enhancement New feature or request

Comments

@cewert
Copy link
Member

cewert commented Oct 5, 2019

As mentioned in #70 the current code base is written for 720p and 1080p. Rewriting for 1080p only and letting Roku auto scale should allow for cleaner easier to read code and prevent future bugs.

As mentioned in the Roku docs - try to make all spacing and layout sizing divisible by 3, which will allow Roku to resize to a lower resolution without glitches.

  • Config Screnes
  • Library select
  • Search results
  • Options
  • All movies list
  • All TV shows list
  • All Collections list
  • Movie details
  • TV show details
  • TV episode list
  • Collections detail
@cfunseth
Copy link
Contributor

cfunseth commented Dec 5, 2019

I'd like to contribute to this project and this seems like a fairly simple issue to start on. I'm not familiar with Roku development so I just want to be sure I'm understand your request. Would this change be as simple as updating the associated XML files for each screen, or are there changes needed for the BrightScript files as well?

@bisby
Copy link
Contributor

bisby commented Dec 5, 2019

Originally some of the code was written assuming there would be variable sizes, so things were written as dimensions * 0.15 (or something like that) to get widths of elements.

If we are writing everything to be a fixed 1080p, we can assert things more confidently, just width = 300 and let Roku do the scaling.

So basically finding all the places where we try to cleverly set sizes on the fly, and then set that statically.

For example:

dimensions = m.top.getScene().currentDesignResolution

Here, we use the dimensions to place the paginator centered at the bottom of the page. dimensions/2 could easily just become 960 instead of doing a calculation every time. (having the values set specifically also makes it easier to tweak)

@cfunseth
Copy link
Contributor

cfunseth commented Dec 5, 2019

Excellent, that helps get me started.

What would the preference for you all be in this instance? Would we want to hard code dimensions in each function, or use references to other scenes like shown in the Pager.brs example with the assumption that the parent node is already following the Roku guidelines?

@bisby
Copy link
Contributor

bisby commented Dec 5, 2019

In most cases hard coded values. "Dimensions" can be assumed.

There might be a few edge cases where something needs to be abstract (if we re-use a component in multiple places and need different params each time, calculating size on the fly makes sense).

But in the Pager example saying translation = [960, 1022] would be enough. And since it's a static value, it can just go into the init() function and we could throw away the entire updateLayout function.

If the [960, 1022] is confusing as to "how we got these numbers", including a comment like "centered full width horizontally, and centered vertically in the bottom 115 pixels" might be useful.

@n76
Copy link
Contributor

n76 commented May 9, 2020

What is the status on this? If I take this as a task would I be in conflict with someone else working on it?

@cewert
Copy link
Member Author

cewert commented May 9, 2020

@n76 I don't think anyone is working on this

#182 made things difficult. The GUI should still be written designed for 1080p but dynamic enough to handle slightly different resolutions (1080p-ish)

@cfunseth
Copy link
Contributor

cfunseth commented May 9, 2020

@n76 I updated the config screen late last year but got so confused by how BrightScript works that I was too nervous to attempt any of the other screens. I haven't worked on it any further so there shouldn't be anything to conflict with.

@n76
Copy link
Contributor

n76 commented May 9, 2020

Okay. No guarantees, no time line, etc. But I'll start playing around with this.

From my first look at things, it seems that if a 1080 screen size is used then a lot of the calculations that are in the BrightScript turn in to constants. And that, in turn means that a fair amount of the object definitions can be moved to the XML which should aid in separating layout design from logic and make things easier in the future.

With respect to issue 182, I think following Roku's advice on safe zones will be needed. That requires some UI design considerations, a bit more than just mechanically replacing computation with constants and moving display objects to XML if it makes things cleaner.

I suspect though that having the UI object definitions cleaned up would make looking at the UI design changes needed to address 182 a bit easer.

@cewert
Copy link
Member Author

cewert commented May 9, 2020

I never seen that link on safe zones good find.

Well if we follow the safe zone advice we should still be able to hard code for 1080p. Exceptions being things like ItemList() being worked on in #206 which should stay dynamic since they are reused for multiple screens.

Edit: Got distracted by your amazing link sorry. Separating logic from layout when possible sounds like a great idea

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

No branches or pull requests

5 participants