Skip to content

atb-agg demo fixes for retina displays#140

Open
yairchu wants to merge 6 commits into
rougier:masterfrom
yairchu:patch-2
Open

atb-agg demo fixes for retina displays#140
yairchu wants to merge 6 commits into
rougier:masterfrom
yairchu:patch-2

Conversation

@yairchu

@yairchu yairchu commented Dec 20, 2016

Copy link
Copy Markdown
Contributor

The atb-agg demo looks like this on my MacBook Pro (Retina, 13-inch, Late 2012):
screen shot 2016-12-20 at 2 01 46 pm
And the atb widgets do not respond to the mouse at their displayed locations.

This change fixes these issues.

@rougier

rougier commented Dec 20, 2016

Copy link
Copy Markdown
Owner

Thanks. Actually, I think all the demos get the same problem on mac book pro retina. I opened issue #91 some time ago but I've never found the time to make a proper fix. Do you have an idea how we could factorize your fix ?

@yairchu

yairchu commented Dec 22, 2016

Copy link
Copy Markdown
Contributor Author

For #91, I believe that the different demos will require different changes (as we don't want them all to be really tiny on high density displays right?), and this PR partially fixes the problem (does it for atb-agg).

Comment thread demos/atb-agg.c Outdated
int pixWidth, pixHeight, winWidth, winHeight;
glfwGetFramebufferSize( window, &pixWidth, &pixHeight );
glfwGetWindowSize( window, &winWidth, &winHeight );
is_scaled = pixWidth > winWidth || pixHeight > winHeight;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

bool to int type conversion. for what reason?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If is_scaled is defined as bool, I get the following compilation error:

demos/atb-agg.c:433:5: error: use of undeclared identifier 'bool'
    bool is_scaled;
    ^

I'm used to using bool in C++, but afaik it isn't available in plain C, which I haven't used in quite a while, so I'm not sure what to use.. What do you suggest?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, Done. Now I know of <stdbool.h> in C99 :)

@bagobor

bagobor commented Dec 22, 2016 via email

Copy link
Copy Markdown

@yairchu

yairchu commented Dec 22, 2016

Copy link
Copy Markdown
Contributor Author

Fixed, thanks.

Comment thread demos/atb-agg.c Outdated
if (is_scaled)
TwDefine("TweakBar "
"size = '560 800' "
"position = '1000 40' ");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's better to calculate correct position by multiplying it on fontscaling factor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright, done now

Comment thread demos/atb-agg.c
"min = 6.0 "
"max = 24.0 "
"step = 0.05 "
"max = 60.0 "

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does it still works fine for non-retina systems?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It should, it will just allow large fonts. If you happen to have such a system, could you check?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

just checked on a friend's laptop and it indeed works fine

@yairchu

yairchu commented Dec 28, 2016

Copy link
Copy Markdown
Contributor Author

@bagobor does it look ok now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants