search

Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
310 stars 134 forks source link

Tabs on Editor Windows not displaying correctly on macOS #787

Closed anthonykelly closed 5 months ago

anthonykelly commented 5 months ago

The Tabs on the Editor windows (Equipment, Fermentables, Hops, etc.) are not displaying correctly on macOS. They show correctly on Ubuntu.

For example, this is how the Equipment Editor windows hows on Ubuntu 22.02 and macOS Sonoma 14.2.1.

image

BTW I've just started using Brewtarget and I'm loving it. Thanks to everyone involved!

matty0ung commented 5 months ago

Sorry to hear you're having problems.

Thanks for the screen shots. Can you confirm what version of Brewtarget you're running? Also, could you have a look and see if there are any ERROR or WARNING lines in the log file? (If you go to Tools > Options > Logging, it should show you the log file directory, and in there should be a file called brewtarget.log.)

anthonykelly commented 5 months ago

Version 3.0.10

I can see no errors reported in the log at all.

This is the Normal logging from starting the app and immediately opening the Equipment Editor (which shows as in the macOS screen capture above):

[17:19:26.515] (3zjq6gw) INFO : Starting Brewtarget v 3.0.10  (app name "brewtarget" ) on  "macOS 14.2"  [main.cpp:195]
[17:19:26.515] (3zjq6gw) INFO : Built at Thu Dec 28 09:22:09 UTC 2023 on darwin for darwin with clang compiler  [main.cpp:198]
[17:19:26.515] (3zjq6gw) INFO : Log directory: "/Users/akelly/Library/Preferences/brewtarget"  [main.cpp:201]
[17:19:26.515] (3zjq6gw) INFO : Using Qt runtime v 5.15.11  (compiled against Qt v 5.15.11 )  [main.cpp:202]
[17:19:26.516] (3zjq6gw) INFO : Configuration directory: "/Users/akelly/Library/Preferences/brewtarget"  [main.cpp:203]
[17:19:26.516] (3zjq6gw) INFO : Data directory: "/Users/akelly/Library/Preferences/brewtarget"  [main.cpp:204]
[17:19:26.516] (3zjq6gw) INFO : void (anonymous namespace)::initResourceDir(QDir &) Determined resource directory is "/Applications/brewtarget_3.0.10.app/Contents/Resources"  [Application.cpp:342]
[17:19:26.516] (3zjq6gw) INFO : Resource directory: "/Applications/brewtarget_3.0.10.app/Contents/Resources"  [main.cpp:205]
[17:19:26.612] (3zjq6gw) INFO : bool Database::load() Known DB drivers:  ("QSQLITE")  [database/Database.cpp:638]
[17:19:26.612] (3zjq6gw) INFO : bool Database::impl::loadSQLite(Database &) dbFileName = " /Users/akelly/Library/Preferences/brewtarget/database.sqlite "
dataDbFileName=" /Applications/brewtarget_3.0.10.app/Contents/MacOS/../Resources/default_db.sqlite "  [database/Database.cpp:251]
[17:19:26.614] (3zjq6gw) INFO : bool Database::impl::loadSQLite(Database &) SQLite version QVariant(QString, "3.39.2")  [database/Database.cpp:303]
[17:19:26.616] (3zjq6gw) INFO : bool Database::impl::updateSchema(Database &, bool *) Schema version in DB: 10 , current schema version in code: 10  [database/Database.cpp:399]
[17:19:27.015] (3zjq6gw) WARNING : Populating font family aliases took 203 ms. Replace uses of missing font family "Sans Serif" with one that exists to avoid this cost.   [:0]

Also here's the full Debug log for the same steps: brewtarget.debug.log

matty0ung commented 5 months ago

Thanks - that's helpful in ruling quite a few things out!

Do you get the same problem on other editors (eg Hop Editor or Yeast Editor)?

anthonykelly commented 5 months ago

Yes, it's the same on all the editors; Equipment was just an example.

matty0ung commented 5 months ago

Ah, OK, that's useful.

I think this is an issue with the Qt framework somehow deciding that the text is too long to display, and so showing "..." instead. Other people report seemingly similar issues on Mac (eg https://stackoverflow.com/questions/72828926/qtablewidgetitem-displays-three-dots-instead-of-full-text and https://forum.qt.io/topic/119371/text-in-qtabbar-on-macos-is-truncated-or-elided-by-default-although-there-is-empty-space).

It may well be some small setting on QTabWidget (eg https://doc.qt.io/qt-5/qtabwidget.html#elideMode-prop) that doesn't matter on Windows / Linux but is important to specify on Mac. I'll have a look to see if I can find something obvious to try, but I don't have a Mac to try things out on, so it may be a question of offering you some patched builds to try...

anthonykelly commented 5 months ago

I'm happy to be an unofficial tester.

anthonykelly commented 5 months ago

I did try to build brewtarget myself on my mac to see if I could help but I get errors at linking stage:

[ 86%] Building CXX object CMakeFiles/Brewtarget.dir/src/main.cpp.o
[ 87%] Linking CXX executable Brewtarget.app/Contents/MacOS/Brewtarget
ld: warning: ignoring duplicate libraries: '/opt/homebrew/lib/libxerces-c.a'
ld: Undefined symbols:
  xalanc_1_12::XPathEvaluator::selectNodeList(xalanc_1_12::NodeRefList&, xalanc_1_12::DOMSupport&, xalanc_1_12::XalanNode*, char16_t const*, xalanc_1_12::XalanElement const*), referenced from:
      XmlRecord::load(xalanc_1_12::DOMSupport&, xalanc_1_12::XalanNode*, QTextStream&) in XmlRecord.cpp.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [Brewtarget.app/Contents/MacOS/Brewtarget] Error 1
make[1]: *** [CMakeFiles/Brewtarget.dir/all] Error 2
make: *** [all] Error 2

It's related to xalan-c which is disabled in the Homebrew repository so I installed xalanc from MacPorts instead. Maybe this is the problem.

I was able to build successfully on my Ubuntu machine (following more or less the same process):

[ 97%] Building CXX object CMakeFiles/brewtarget.dir/brewtarget_autogen/mocs_compilation.cpp.o
[ 97%] Building CXX object CMakeFiles/brewtarget.dir/src/main.cpp.o
[ 98%] Linking CXX executable brewtarget
[ 98%] Built target brewtarget
[ 98%] Automatic MOC and UIC for target brewtarget_tests
[ 98%] Built target brewtarget_tests_autogen
[ 99%] Building CXX object CMakeFiles/brewtarget_tests.dir/brewtarget_tests_autogen/mocs_compilation.cpp.o
[ 99%] Building CXX object CMakeFiles/brewtarget_tests.dir/src/unitTests/Testing.cpp.o
[100%] Linking CXX executable bin/brewtarget_tests
[100%] Built target brewtarget_tests
[100%] Final Message
⭐⭐⭐ Finished compiling Brewtarget.  Please run  sudo make install  to install locally. ⭐⭐⭐
[100%] Built target FinalMessage

Unfortunately I'm not skilled enough to diagnose what's wrong with the mac build but if I could get it working I could try out some changes very quickly.

matty0ung commented 5 months ago

I feel your pain on Mac builds! But there is help at hand. You actually just need to follow what's done in the automated builds (see https://github.com/Brewtarget/brewtarget/blob/develop/.github/workflows/mac.yml).

For the reasons you say, you need to have both MacPorts and Homebrew installed. With that starting point, if you then install Python (eg brew install python@3.12) you can run the bt build script and tell it to install all the other dependencies and sets up a Meson build environment. It's just a question of running ./bt setup all in the brewtarget directory.

At that point you should then be able to do either a CMake build (in the build directory) or a Meson build (in the mbuild directory). (Note that with the CMake build you need to first do export PATH=/usr/local/opt/qt5/bin:$PATH.)

anthonykelly commented 5 months ago

I got it built! I just needed to remove the homebrew xerces-c and replace it with xercesc3 from MacPorts.

I'm really in over my depth here but I tried adding the line this->tabWidget_editor->setElideMode(Qt::ElideNone); to EquipmentEditor::EquipmentEditor(QWidget* parent, bool singleEquipEditor) and it resulted in this:

image

Still not looking pretty but the text is appearing at least.

I can continue poking around but if there's anything you'd like me to try just let me know.

matty0ung commented 5 months ago

Cool! That's already progress!

Have a play around with https://github.com/Brewtarget/brewtarget/blob/develop/ui/equipmentEditor.ui Sometimes removing things like sizepolicy helps, because you then let Qt decide what's best instead of trying to override it -- at least that's what I've found in other places.

anthonykelly commented 5 months ago

I've tried fiddling with almost every property in UI Designer and nothing makes the actual Tab size increase. The only thing that helps at all is setting elideMode = ElideNone and that just lets the text overflow the tiny tabs.

image

I saw online a suggestion to put something like QTabBar::tab { height: 200px; width: 100px; } into the stylesheet property to change the size of the actual Tab bar within the widget but that too had no effect.

I'm starting to suspect a bug in Qt on macOS, perhaps something broke in the latest Sonoma upgrade.

matty0ung commented 5 months ago

Sounds like you've tried a lot of the things I would have! A couple of further ideas:

anthonykelly commented 5 months ago

The Options windows does display correctly:

image

I guess this is encouraging although the only difference I can see in the properties is the ElideNone change I made. image

Looking at the Equipement UI file after using Qt Designer it has 23 extra rows. I'll revert to the original file and use a text editor from now on. Qt Designer is handy just to see what parameters and options are there.

I'll see if I can change the Priming Dialog to use QTabWidget.

anthonykelly commented 5 months ago

Changing the Priming Dialog to use QTabWidget worked too.

image image

Curiouser and curiouser.

matty0ung commented 5 months ago

That's good progress! What happens on the altered Priming Dialog if you change the tabPosition property on the QTabWidget from QTabWidget::North to QTabWidget::West (ie to put the tabs at the side rather than the top)?

anthonykelly commented 5 months ago

Still looks good on West...

image
matty0ung commented 5 months ago

That's encouraging - and interesting to see the text orientation on those side tabs. Maybe MacOS doesn't like us trying to force side tab text to be horizontal?

At this point, I'd be tempted to start copying tabs and input fields over from, say Equipment Editor to Priming Dialog. Obviously the fields will be empty/unconnected, but I'm thinking if we could gradually turn Priming Dialog into Equipment Editor from a layout perspective (probably keeping an extra tab for the real fields of Priming Dialog so nothing breaks!) then we'd hopefully find the point at which things break.

anthonykelly commented 5 months ago

Yes, when I move the Tab Widget from Equipment Editor to Priming Dialog it displays correctly.

image

It soon crashes but I guess that's not unexpected... 

[10:26:09.271] (3ou41s0) ERROR : ASSERT: "this->pimpl->m_initialised" in file /Users/akelly/brewtarget/src/widgets/SmartLabel.cpp, line 242  [widgets/SmartLabel.cpp:242]
fish: Job 1, 'brewtarget.app/Contents/MacOS/B…' terminated by signal SIGABRT (Abort)

You suggested that maybe MacOS doesn't like us trying to force side tab text to be horizontal. Is there somewhere in the code where you do that that I could comment out and see what happens?

The problem is on all of these Editors: Style, Equipment, Fermentables, Hops, Miscellaneous and Year. The Tab Widget shows fine in Options and the main Recipe Editor.

matty0ung commented 5 months ago

You suggested that maybe MacOS doesn't like us trying to force side tab text to be horizontal. Is there somewhere in the code where you do that that I could comment out and see what happens?

Yes, good idea. In the src/EquipmentEditor.cpp and similar files, comment out the line that says this->tabWidget_editor->tabBar()->setStyle(new BtHorizontalTabs);. If that fixes things, then we can have a closer look at src/BtHorizontalTabs.h (which is likely inspired by https://forum.qt.io/topic/86186/horizontal-text-in-qtabwidget).

anthonykelly commented 5 months ago

Eureka!

image

Commenting out that line did the trick.

matty0ung commented 5 months ago

Excellent!

Next step is probably to have a proper look at BtHorizontalTabs.h. There may be other ways to achieve the same end (eg different code snippet at https://stackoverflow.com/questions/14299651/qtabwidget-tabs-on-the-vertical-but-text-in-horizontal). Sometimes with sizing things in Qt, you have to jump through a few hoops, and there are some extra ones to cope properly with high DPI displays.

anthonykelly commented 5 months ago

I couldn't come up with anything in BtHorizontalTabs.h that helped. What's in there now is definitely causing the problem on macOS.

However, this small change to EquipmentEditor.cpp ignores BtHorizontalTabs and always uses the North tab position on macOS bypasses the issue. 

   // If macOS always use North
   if (QSysInfo::productType() == "osx")
      this->tabWidget_editor->setTabPosition(QTabWidget::North);
   else
      this->tabWidget_editor->tabBar()->setStyle( new BtHorizontalTabs );
image

You may be able to come up with some change to BtHorizontalTabs.h that fixes the issue on macOS but this would be an acceptable fix for me.

matty0ung commented 5 months ago

After a bit of digging, I think I now understand what BtHorizontalTabs.h is attempting to do. Will have a look at adding some diagnostics and/or an alternative approach. At the limit, as you say, we may be able to have a Mac-specific behaviour that moves the tabs up to the top of the widget instead of rotating them.

matty0ung commented 5 months ago

OK, so I've done a minimalistic fix. I didn't totally get to the bottom of why the tabs get drawn too small on Mac when we "unrotate" them back to horizontal via the BtHorizontalTabs, and I also didn't find a simple way to fix things. After thinking about it, I was hesitant to move side tabs to the top in case that ends up breaking something else (either now or at a future date). Instead, I've just left the side tabs in "default behaviour" on Mac, which hopefully works.

Also added some special case handling so that the rotated side tabs that do work on Mac (the icons) still work!

anthonykelly commented 5 months ago

Thanks @matty0ung. I've built the latest from master on macOS and I can confirm the tabs look good now (still good on Ubuntu too). Do you want me to close this issue now?

image
matty0ung commented 5 months ago

Excellent - many thanks. Leave the issue open just for a short while longer. I'll close it when I do the 3.0.11 release (hopefully today!).

matty0ung commented 5 months ago

Release went smoothly for once! :smile:

Fix included in https://github.com/Brewtarget/brewtarget/releases/tag/v3.0.11.