Skip to content

Add retina display support for BOINC Manager#3473

Merged
CharlieFenton merged 1 commit intoBOINC:masterfrom
brianclinkenbeard:patch-1
Mar 3, 2020
Merged

Add retina display support for BOINC Manager#3473
CharlieFenton merged 1 commit intoBOINC:masterfrom
brianclinkenbeard:patch-1

Conversation

@brianclinkenbeard
Copy link
Contributor

Fixes #3383

Please note: I am currently unable to build BOINC using Xcode 11.4 for Catalina due to a build error with WxWidgets, so I cannot test this change. Until I can fix this issue, if anyone with retina hardware is able to build and test this I would appreciate it.

Description of the Change
This simply adds the NSPrincipleClass key to Info.plist to allow retina support as described here.

Alternate Designs
This may need to be added to other .plist files in clientgui/mac/templates to offer retina support for the installer, screensaver, etc. Without being able to test this I don't know.

Release Notes
Add support for macOS retina displays.

@AenBleidd
Copy link
Member

@CharlieFenton, @brevilo, could you please build and test this fix?

@brevilo
Copy link
Contributor

brevilo commented Feb 27, 2020

I have no build infrastructure for that so it will take some time...

@brevilo
Copy link
Contributor

brevilo commented Feb 27, 2020

I am currently unable to build BOINC using Xcode 11.4 for Catalina due to a build error with WxWidgets

Are these errors already logged anywhere?

@brevilo
Copy link
Contributor

brevilo commented Feb 27, 2020

Sorry, I'm also seeing errors left and right (#3474, #3475, #3476, #3477) so I can't test this PR right now.

@brevilo
Copy link
Contributor

brevilo commented Feb 27, 2020

@brianclinkenbeard : see this to get (at least) your wxWidgets building again.

Copy link
Contributor

@brevilo brevilo left a comment

Choose a reason for hiding this comment

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

Back to topic: I'm sorry, but the change seems to be ineffective. I don't see any improvement.

I'm wondering, though. Should this template be reflected in the Manager's application bundle's Info.plist? If so, the new key/value pair can't be found. OTOH, the CFBundleVersion also differs from the template which might hint at the template not being used at all...?

Update:
The template didn't get used after I pulled your changes because it just isn't updated if it already exists in the build directory! IMHO, that's another flaw in the build system.

Anyhow, your change is in fact effective - YEAH!
But it probably breaks things anyway - BOOO!

Apparently the changed resolution triggers an assertion at BOINCTaskBar.cpp:451. Since that's related to pixel formats and anti-aliasing I think this could indeed be related to your change, not the wxWidgets 3.1.3 upgrade et al. This needs to be looked into by someone more knowlegable in that area.

Cheers

PS: The in-app icons might need a refresh as well now that the main window elements and fonts are rendered nicely again.

Copy link
Contributor

@brevilo brevilo left a comment

Choose a reason for hiding this comment

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

So, I re-checked this after @CharlieFenton patched the build to get wx 3.1.0 working again. With wx 3.1.0 I don't see the assertion triggered, so that's a problem for later when we upgrade wxWidgets to >=3.1.3. Until then I'm fine merging this PR.

@CharlieFenton : do you want to sign-off on this one yourself? Otherwise I'd merge it.

@CharlieFenton
Copy link
Contributor

I will merge now. @brianclinkenbeard: I'm impressed that you studied the code enough to understand that the plist files are generated dynamically at run time by clientgui/mac/setversion.cpp so as to properly update the BOINC version number. As I have been stepping away from BOINC (or at least trying to) after many years, I welcome your help. Thank you.

(Don't forget to delete your patch-1 feature branch.)

@CharlieFenton CharlieFenton merged commit 524dc20 into BOINC:master Mar 3, 2020
@brevilo
Copy link
Contributor

brevilo commented Mar 3, 2020

Hm, I just tried to check simple view and it doesn't work for me. @CharlieFenton can you test that on your rig as well please? If it's broken due to this PR we need to revert the merge and reopen the PR...

Update:

By "doesn't work" I mean that the manager seems to close itself when switching to simple view. I somehow managed to get it working once (and it looked ok) but know the manager can't be started anymore as it closes itself right away which is consistent with switching.

@brevilo
Copy link
Contributor

brevilo commented Mar 3, 2020

Also, while improving the wx GUI it doesn't fully fix #3383 in my opinion. The contents of the "Statistics" and "Disk" tabs as well as all in-app icons are still blurry and I couldn't test Simple View at all (see previous comment).

@brevilo
Copy link
Contributor

brevilo commented Mar 3, 2020

Ok, this PR definitely causes a segfault in wx itself:

wxWidgets-3.1.0/src/common/sizer.cpp(2100): assert "!(flags & wxALIGN_RIGHT)" failed in DoInsert(): Horizontal alignment flags are ignored in horizontal sizers
SIGSEGV: segmentation violation

relevant stacktrace:

37  BOINCManager                        0x000000010c5e71ac wxOnAssert(char const*, int, char const*, char const*, char const*) + 172
38  BOINCManager                        0x000000010c94a273 wxBoxSizer::DoInsert(unsigned long, wxSizerItem*) + 515
39  BOINCManager                        0x000000010c485d54 wxSizer::Insert(unsigned long, wxSizerItem*) + 52
40  BOINCManager                        0x000000010c37e730 wxSizer::Add(wxSizerItem*) + 64
41  BOINCManager                        0x000000010c37a453 wxSizer::Add(wxWindow*, int, int, int, wxObject*) + 115
42  BOINCManager                        0x000000010c587e46 CSimpleTaskPanel::CSimpleTaskPanel(wxWindow*) + 4854
43  BOINCManager                        0x000000010c588b9d CSimpleTaskPanel::CSimpleTaskPanel(wxWindow*) + 29
44  BOINCManager                        0x000000010c5b4e8f CSimpleGUIPanel::CSimpleGUIPanel(wxWindow*) + 1647
45  BOINCManager                        0x000000010c5ae4fd CSimpleGUIPanel::CSimpleGUIPanel(wxWindow*) + 29
46  BOINCManager                        0x000000010c5ae45d CSimpleFrame::CSimpleFrame(wxString, wxIconBundle*, wxPoint, wxSize) + 301
47  BOINCManager                        0x000000010c5ae53d CSimpleFrame::CSimpleFrame(wxString, wxIconBundle*, wxPoint, wxSize) + 45
48  BOINCManager                        0x000000010c3a53f6 CBOINCGUIApp::SetActiveGUI(int, bool) + 2054
49  BOINCManager                        0x000000010c3a3f2e CBOINCGUIApp::OnInit() + 9182

I'll revert the merge...

@brevilo
Copy link
Contributor

brevilo commented Mar 3, 2020

@CharlieFenton, please review and merge #3488 as soon as you can.

@brevilo
Copy link
Contributor

brevilo commented Mar 3, 2020

Ok, sorry for the confusion but looking further into this it seems that this PR IS NOT the culprit. Reverting it DOES NOT get rid of the assertion/segfault. My current working hypothesis is that the earlier patch against wx 3.1.0 is insufficient and we probably need to revert (or fix) that one instead.

@CharlieFenton can you please confirm that?

@CharlieFenton
Copy link
Contributor

@brevilo: you are seeing the asserts because you are running a development build of wxWidgets. Those asserts have been there for years, and are due to minor inconsistencies in the use of sizers. Those inconsistencies don't seem to cause any operational problems, so no one has ever bothered to fix them; we've been treating them as warnings rather than errors.

But I've never seen the asserts cause a segfault. I've been testing a release (deployment) build with @brianclinkenbeard's patch and SimpleVView works correctly.

Note that this patch is expected to only fix the rendering of text. The link @brianclinkenbeard provided says:

To have text rendered sharply on macs with a Retina display the key "NSPrincipalClass" should exist in the info.plist of your application. Its value seems to be irrelevant - probably it's best to just set it to the default "NSApplication".

Fixing graphic elements such as icons requires providing high-resolution artwork and takes considerably more effort. So I agree that we have only partially satisfied #3383. I was surprised that GitHub says I closed #3383; I did not do this explicitly so someone else must have linked these 2 PRs.

@brevilo
Copy link
Contributor

brevilo commented Mar 3, 2020

you are seeing the asserts because you are running a development build of wxWidgets.

Ok, I see. Never mind then. This should really be fixed as it's obviously tribal knowledge only and confuses those unchristened.

I've been testing a release (deployment) build with @brianclinkenbeard's patch and SimpleVView works correctly.

Ok, as long as that includes your latest Catalina/Xcode 11 patches for wx 3.1.0 we should be good.

I was surprised that GitHub says I closed #3383

That's done automatically because of the "Fixes" tag in the PR's description.

@CharlieFenton
Copy link
Contributor

I just tried running a development build with @brianclinkenbeard's patch. I get the wxWidgets assert as usual. The resulting dialog offers 3 options:

Do you want to stop the program? You can also choose [cancel] to suppress further warning.

Pressing yes stops the program, though I did not get a backtrace. Pressing cancel brings up the Simple View.

@brianclinkenbeard
Copy link
Contributor Author

brianclinkenbeard commented Mar 5, 2020

I wouldn't expect this change to cause any build or assert errors, since it simply changes the Info.plist to tell the operating system the application is retina capable.

Indeed, this change will not have any effect on graphics. High DPI images would have to be added to render correctly on newer Mac displays.

@brianclinkenbeard brianclinkenbeard deleted the patch-1 branch March 8, 2020 11:01
@AenBleidd AenBleidd removed this from the Client/Manager 8.0 milestone Jul 10, 2020
@AenBleidd AenBleidd added this to the Client Release 7.16.8 milestone Jul 10, 2020
@AenBleidd AenBleidd modified the milestones: Client Release 7.18.0, Client Release 7.16.11 Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BOINC Manager for macOS should be optimized for Retina displays

4 participants