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

Replace images in wxRendererMac::DrawTitleBarBitmap() with drawing code. #104

Merged
merged 3 commits into from Jan 5, 2016

Conversation

TcT2k
Copy link
Contributor

@TcT2k TcT2k commented Sep 25, 2015

The low resolution (14x14) bitmaps scaled badly on high resolution displays. A close button suitable for usage inside a window (like wxInfoBar) is not available via HI theme drawing methods. This drawing code tries to emulate a close button, as close as possible to the one found in the Xcode 6+ welcome window.

Normal: screen shot 2015-09-25 at 15 20 33 Hover: screen shot 2015-09-25 at 15 20 41 Old implementation: screen shot 2015-09-25 at 15 43 48

@TcT2k
Copy link
Contributor Author

TcT2k commented Oct 6, 2015

@vadz previous comments where lost when I pushed the updated implementation, so I'll reply here:
I wasn't even ware that GetContentScaleFactor() returns something on MSW. I've updated the second commit to include other ports too. It looks much better now on Windows. But a little bit of platform specific code is still required as only the OSX version of wxBitmap has ctors with a specifiable content scaling factor.

Windows 8.1 (200%):
wxwidgets_closebtn_win

dc.SetPen(wxPen(glyphColor, 1 * contentScale));

wxRect centerRect(rect);
centerRect.Inflate(-5 * contentScale);
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be written a bit more clearly as Deflate(5*contentScale).

@vadz
Copy link
Contributor

vadz commented Jan 2, 2016

Sorry for the delay, I've finally reviewed the latest version of the patch. If the latest comments could be addressed (especially the one about CreateScaled()), I promise to apply this without any further delay. TIA!

@TcT2k
Copy link
Contributor Author

TcT2k commented Jan 4, 2016

I've integrated your comments, but I wasn't able to use wxBitmap::CreateScaled() (which is currently undocumented) as the result was an empty bitmap. I'm not really sure why as the code looks correct when stepping into it. It would obviously be cleaner without the #ifdef but I couldn't find a way to make it work without the cocoa specific ctor.

@vadz
Copy link
Contributor

vadz commented Jan 5, 2016

It would be great if somebody with a retina Mac could debug this, it's really appalling that we don't have a way to create a bitmap in a portable way :-(

Also, sorry to be a pain, but what about the hardcoded RGB values? They seem to be there without any explanations...

The low resolution (14x14) bitmaps scaled badly on high resolution displays. A close button suitable for usage inside a window (like wxInfoBar) is not available via HI theme drawing methods. This drawing code tries to emulate a close button, as close as possible to the one found in the Xcode 6+ welcome window.
@TcT2k
Copy link
Contributor Author

TcT2k commented Jan 5, 2016

@vadz
I've misinterpreted CreateScaled. I thought it would create a scaled bitmap based and the current content, but obviously it creates the bitmap object like other Create methods.
The code now works without ifdefs. I've tested OS X and Win 8.1 with a 4K display and the results match the screenshots posted above.

As for the RGB values I've included a comment that they are hard coded based on the Xcode 6+ welcome screen. I haven't found a good match using any system colors, which would obviously be a nicer approach.

I've also added documentation for CreateScaledin an additional commit.

Bitmap for the close button is scaled based on GetContentScaleFactor(). On OS X the resulting bitmap has to use the correct scaling factor in order to be displayed correctly on the button when using a high resolution display.
The two methods were previously undocumented but might be of interest
to user code for high DPI display compatibility.
@vadz vadz merged commit 08dc940 into wxWidgets:master Jan 5, 2016
@vadz
Copy link
Contributor

vadz commented Jan 5, 2016

Great, thanks for fixing this!

@TcT2k TcT2k deleted the osx_close_btn branch October 21, 2016 06:37
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.

None yet

2 participants