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

rqt_bag does not position icons correctly on OS X #68

Merged
merged 1 commit into from Aug 8, 2013
Merged

rqt_bag does not position icons correctly on OS X #68

merged 1 commit into from Aug 8, 2013

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented Aug 7, 2013

@ablasdel
Copy link
Contributor

ablasdel commented Aug 2, 2013

This must be a bug in Qt on OS X since these items are just fixed size buttons in a horizontal layout. Nothing fancy is going on. I've read that OS X qt sometimes handles drawing a bit differently than other platforms but this seems to be a bit over the top. Unfortunately I haven't found any reference to this kind of issue specifically.

@wjwwood
Copy link
Member Author

wjwwood commented Aug 5, 2013

The refresh icon is not sized properly:

screen shot 2013-08-05 at 4 46 43 pm

@ablasdel
Copy link
Contributor

ablasdel commented Aug 5, 2013

This bug might be better represented on the rqt package since, as you have just shown with that shot, it is not at the rqt_bag level. I will make a bug that links here.

@ablasdel
Copy link
Contributor

ablasdel commented Aug 6, 2013

I am closing this bug as a duplicate of ros-visualization/rqt#83 because it is better tracked in ros-visualization/rqt since it is a global rqt (really qt) problem.

@ablasdel ablasdel closed this Aug 6, 2013
@wjwwood
Copy link
Member Author

wjwwood commented Aug 6, 2013

If I unset the maximum size for the buttons in Qt Designer it looks more reasonable, but the buttons are still huge:

screen shot 2013-08-05 at 6 02 16 pm

@wjwwood
Copy link
Member Author

wjwwood commented Aug 6, 2013

(I added that CheckBox while testing)

@ablasdel
Copy link
Contributor

ablasdel commented Aug 6, 2013

This looks good honestly. I'll check your fork on Ubuntu and if it looks ok I think this is an improvement on before.

@ablasdel
Copy link
Contributor

ablasdel commented Aug 6, 2013

Reopening because we can (hopefully) work around the bug specifically for rqt_bag!

@wjwwood can you try this commit hash and give me a screenshot of what it looks like for you on osx?
ablasdel@24fd942

On ubuntu it looks like this:
rqt_bag_ubuntu

@wjwwood
Copy link
Member Author

wjwwood commented Aug 7, 2013

I upgraded this to a pull request, can you test this on Ubuntu, this is what it currently looks like on OS X:

screen shot 2013-08-07 at 4 58 28 pm

@ablasdel
Copy link
Contributor

ablasdel commented Aug 8, 2013

This merge makes the buttons a touch wider on ubuntu but that is fine.
screenshot from 2013-08-07 17 04 17

I'm a bit worried about your last 4 buttons not having any icons. Do they still work?

@dirk-thomas
Copy link
Contributor

Let me compare this with other ui files before the merge.

@ablasdel
Copy link
Contributor

ablasdel commented Aug 8, 2013

@ablasdel
Copy link
Contributor

ablasdel commented Aug 8, 2013

I removed the "maximumSize" property entirely and it behaves the same as that pull request.

@dirk-thomas
Copy link
Contributor

By default the ui file does not contain any properties. So instead of setting the values to 16777215 you should indeed "reset" that property in QtCreator which will effectively remove the property from the ui file. Please update the pull request accordingly.

@ablasdel
Copy link
Contributor

ablasdel commented Aug 8, 2013

I've done that in this commit
ablasdel@15ea675

That way we don't have to waste anymore of williams precious time.

@ablasdel
Copy link
Contributor

ablasdel commented Aug 8, 2013

If I merge this will it merge william's and mine?
new and interesting git questions every day.

@ablasdel
Copy link
Contributor

ablasdel commented Aug 8, 2013

I'll merge this at ~7-8ish if there is no complaint here by then.

Thanks for the help will/dirk

@wjwwood
Copy link
Member Author

wjwwood commented Aug 8, 2013

Updated.

This allows buttons to be sized correctly on
OS X
@dirk-thomas
Copy link
Contributor

Looks good now. I will do the merge...

dirk-thomas added a commit that referenced this pull request Aug 8, 2013
rqt_bag does not position icons correctly on OS X
@dirk-thomas dirk-thomas merged commit ac2c7b5 into ros-visualization:hydro-devel Aug 8, 2013
@wjwwood wjwwood deleted the fix_icon_osx branch August 19, 2013 20:49
@jf---
Copy link

jf--- commented Jan 11, 2014

To what extent might this be Retina / HighDPI related?
This is known to be a bit buggy on Qt 4.10

@dirk-thomas
Copy link
Contributor

I think what you are referring to is more related to #83. We have added an explicit iconSize (eafed12). Even if 16x16 should be default value on OS X it did not work correctly without setting the iconSize explicitly.

@jf---
Copy link

jf--- commented Jan 11, 2014

dirk, indeed clearly not. thanks helping me get up to speed, sorry for the noise.

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

Successfully merging this pull request may close these issues.

None yet

4 participants