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

various screen fixes #423

Merged
merged 6 commits into from Jun 20, 2018
Merged

various screen fixes #423

merged 6 commits into from Jun 20, 2018

Conversation

@tehn
Copy link
Member

@tehn tehn commented Jun 12, 2018

fixes #412

fixes #410

fixes #395

also adds screen.pixel(x,y) (lua)

@tehn
Copy link
Member Author

@tehn tehn commented Jun 12, 2018

@tehn
Copy link
Member Author

@tehn tehn commented Jun 12, 2018

added one more fix to menu display

fixes #416

@tehn tehn requested a review from markwheeler Jun 12, 2018
@tehn
Copy link
Member Author

@tehn tehn commented Jun 12, 2018

so i think i got everything. unfortunately you don't have hardware at the moment to check!

@markwheeler
Copy link
Collaborator

@markwheeler markwheeler commented Jun 12, 2018

Ha yep, actually I don't have the build environment setup anyway (only been doing lua and sc stuff so far).

Looking at the code the fixes look like they make sense.

However I would suggest also removing the half pixel offset from screen_arc, screen_rect, screen_move and screen_line. The offset only really makes sense if you are drawing with an odd-width stroke which seems like a confusing assumption to make. Thoughts?

@tehn
Copy link
Member Author

@tehn tehn commented Jun 12, 2018

FYI the dev chain is all built in on norns. to recompile:

cd norns
git pull
./waf
./stop.sh
./start.sh

and you'll be up and running.

perhaps i'm misunderstanding cairo--- i added the offsets after some googling as a result of initial weird rendering issues.

is pixel 0,0 translated as the area between (-0.5,-0.5) and (0.5,0.5) or is it (0,0) to (1,1)?

looking for references now...

@markwheeler
Copy link
Collaborator

@markwheeler markwheeler commented Jun 12, 2018

I think this explains it: https://www.cairographics.org/FAQ/#sharp_lines

and good to know about the recompiling thanks!

@tehn
Copy link
Member Author

@tehn tehn commented Jun 12, 2018

thanks for the link, that was the one i read way in the past and had forgotten.

so you imagine it's best to let users figure out the offset business on their own? as the current hack isn't a real solution clearly.

@markwheeler
Copy link
Collaborator

@markwheeler markwheeler commented Jun 12, 2018

Oh I didn't answer your earlier question: When Cairo draws a 1px rect with top left at '0,0' it is exactly filling the first pixel, which could be described as the area from 0-1 on a graph.

Yes I think if we stick to the default cairo way of doing things we'll at least be in alignment with how most screen drawing systems work. And yep I'm sure some folks will draw 1 pixel lines and wonder why they're blurry but at least when they end up at that link or similar it'll get them on track!

Of course this change will throw off some of the current UI, we'll have to do a pass to see what needs tweaking.

@tehn
Copy link
Member Author

@tehn tehn commented Jun 12, 2018

checking the UI now. rect is really off with integers using stroke so i'm offsetting the LEVELS page with the bars. other scripts with normals lines seem fine ie awake and playfair

@markwheeler
Copy link
Collaborator

@markwheeler markwheeler commented Jun 12, 2018

Hmm I would have expected any on-pixel line drawing to now need to be shifted on the script side (eg, in awake) in order to stay sharp. Think I need to test this on device! :D

@tehn
Copy link
Member Author

@tehn tehn commented Jun 12, 2018

Hmm I would have expected any on-pixel line drawing to now need to be shifted on the script side (eg, in awake) in order to stay sharp. Think I need to test this on device! :D

i expected it as well, but playfair and awake look identical to as before.

@tehn
Copy link
Member Author

@tehn tehn commented Jun 12, 2018

scratch that... almost identical. there's some off-by-one pixel tuning now but it's minor

@tehn
Copy link
Member Author

@tehn tehn commented Jun 12, 2018

i think this is good to merge if you (or anyone) would like to approve the PR

@pq
pq approved these changes Jun 12, 2018
Copy link
Collaborator

@pq pq left a comment

image

@tehn tehn merged commit 60f736e into master Jun 20, 2018
@tehn tehn deleted the screen-fix branch Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants