Created attachment 197838 [details] Screenshot showing the problem When using Windows, Plasma (kf6) or the "gen" VCL plugin, the new Find sidebar uses a constant height for its items, which makes the size of everything look weird. This works fine in gtk3 because it use native widgets. However, other VCL plugins will use SvImpLBox, which uses a constant height for its entries. The attached screenshot compares gtk3 (left, looks good) and kf6 (right, looks bad). Code pointer: See SvImpLBox::Paint where it does: nEntryHeight = m_pView->GetEntryHeight(); And then use this height for everything. System info Version: 24.8.2.1 (X86_64) / LibreOffice Community Build ID: 480(Build:1) CPU threads: 16; OS: Linux 6.11; UI render: default; VCL: kf6 (cairo+wayland) Locale: pt-BR (pt_BR.UTF-8); UI: en-US Ubuntu package version: 4:24.8.2-0ubuntu1 Calc: threaded Also present in current 25.2 master
@Michael, what is your opinion about this issue? I tried changing this code to make it use the size of each entry instead, but I'm having lots of side effects. For kf5/kf6 a solution would be to use native QTreeView. I noticed that in the code we have a prototype for implementing QTreeView in QtInstanceTreeView.cxx (for qt5/qt6). Are you planning to work on this? How hard would it be to implement this? Maybe I could give it a try, but I would need some mentoring.
(In reply to Rafael Lima from comment #1) > @Michael, what is your opinion about this issue? > > I tried changing this code to make it use the size of each entry instead, > but I'm having lots of side effects. > > For kf5/kf6 a solution would be to use native QTreeView. Indeed, I also see the 2 options you mention: 1) adjusting the vcl Widget (you mentioned SvImpLBox), which would currently apply for everything except gtk3/gtk4 2) for Qt-based VCL plugins: using a native QTreeView, which at least I would consider the mid- to long-term goal anyway > I noticed that in the code we have a prototype for implementing QTreeView in > QtInstanceTreeView.cxx (for qt5/qt6). > > Are you planning to work on this? I plan to continue implementing more use of native Qt widgets (Qt welding, see tdf#130857), and actually started implementing a bit more of the QtInstanceTreeView methods, initially focusing on relatively simple dialogs using tree views like Calc's "Sheet" -> "Show Sheet" dialogs or the "Past Special" dialog. The idea is to eventually use native Qt widgets for most of the UI, similar to what happens with gtk3/gtk4 already. > How hard would it be to implement this? Maybe I could give it a try, but I > would need some mentoring. Help is appreciated of course, and I'm happy to provide more information - please just ask. The approach I'm using so far is basically looking at one dialog at a time and implement the corresponding weld API in the vcl/qt5/QtInstance* classes that that dialog uses/needs, starting with simpler dialogs and then slowly moving to more complex ones (that require more of the implementation being done). So what I'm doing usually looks approximately like this: 1) choose a dialog to tackle next 2) add that dialog to the list of supported dialogs in `QtInstanceBuilder::IsUIFileSupported` locally 3) run LO with the qt6 VCL plugin and environment variable SAL_VCL_QT_USE_WELDED_WIDGETS=1 set (to enable use of native Qt widgets) 4) open the dialog This will usually already trigger asserts in methods that are used, but not implemented yet. If so: Implement what's needed (and sometime I implement more of the methods in the corresponding classes, while at it anyway). Once the dialog opens: Check whether the dialog works as expected and address any issues. Potentially check the dialog implementation to see whether there's anything else that might need attention. A `git log --grep=tdf#130857` shows sample commits that might give an impression. `QtInstanceBuilder::IsUIFileSupported` lists the dialogs already supporting native Qt widgets, and a git blame will show the commits adding them, with each commit message also mentioning how to trigger the corresponding dialog. (AFAICT, the dialogs should be fully functional, but in order to look really appealing, more details like handling of margins, alignment, text attributes,... still might need to be implemented). I cannot really estimate how much effort the "Find" sidebar would be. Other than the dialogs I've been looking at so far, it might require implementing support for ~everything that the main window uses first, as the sidebar is part of that one, so would - at least with the approach I'm taking so far - probably be something looked at quite a bit in the future still, once more of the weld API has been implemented in Qt. (Maybe there is a way to also use a native QTreeView in a window also containing vcl widgets, but that's not what I have been looking into so far.) Sahil mentioned he's planning to also work on an implementation of the weld API for Windows, using the Windows API (controls). Does that help? What do you think? Version: 25.2.0.0.alpha1+ (X86_64) / LibreOffice Community Build ID: ff3d9ab6ecda2a1b8a3e7d39865cf0dbde916df6 CPU threads: 32; OS: Linux 6.11; UI render: default; VCL: qt6 (cairo+wayland) Locale: en-GB (en_GB.UTF-8); UI: en-US Calc: CL threaded
(In reply to Michael Weghorn from comment #2) > I cannot really estimate how much effort the "Find" sidebar would be. Other > than the dialogs I've been looking at so far, it might require implementing > support for ~everything that the main window uses first, as the sidebar is > part of that one, so would - at least with the approach I'm taking so far - > probably be something looked at quite a bit in the future still, once more > of the weld API has been implemented in Qt. > (Maybe there is a way to also use a native QTreeView in a window also > containing vcl widgets, but that's not what I have been looking into so far.) Another thought: Since the sidebar can be undocked, an idea might be to try doing that to see what it looks like once all controls used for the sidebar have been welded - at least for testing (and then avoiding to have to support the full main window at that point of testing already, if it reduces the work needed).
(In reply to Michael Weghorn from comment #2) > So what I'm doing usually looks approximately like this: > ... I'll try that with the Find sidebar... maybe I'll try something simpler first (some other smaller dialog) just for learning. > (Maybe there is a way to also use a native QTreeView in a window also > containing vcl widgets, but that's not what I have been looking into so far.) I did not know that all controls in the same window needed to be supported. I always thought that implementing a single control would be enough. > Does that help? What do you think? It does... now I know where to start. I'll keep you up-to-date on my progress. > Another thought: Since the sidebar can be undocked, an idea might be to try doing > that to see what it looks like once all controls used for the sidebar have been > welded - at least for testing (and then avoiding to have to support the full main > window at that point of testing already, if it reduces the work needed). I had not anticipated this complication about the sidebar. I tried the approach you mentioned (with the undocked sidebar) and got this: warn:vcl.qt:15757:15757:vcl/qt6/../qt5/QtBuilder.cxx:411: QtBuilder::applyPackingProperties not yet implemented for this case warn:vcl.qt:15757:15757:vcl/qt6/../qt5/QtBuilder.cxx:241: Widget type not supported yet: GtkToolbar soffice.bin: /home/rafael/Programming/libreoffice/vcl/qt6/../qt5/QtBuilder.cxx:243: QObject* QtBuilder::makeObject(QObject*, std::u16string_view, const rtl::OUString&, BuilderBase::stringmap&): Assertion `false && "Widget type not supported yet"' failed. I'm guessing GtkToolbar will have to be first.
(In reply to Rafael Lima from comment #4) > I'll try that with the Find sidebar... maybe I'll try something simpler > first (some other smaller dialog) just for learning. Sounds great. If you have any questions or ideas how to do things better as you have to deal with existing code, please don't hesitate to bring that up. > I did not know that all controls in the same window needed to be supported. > I always thought that implementing a single control would be enough. There might be some way to make mixing VCL and native widgets work somehow, e.g. `GtkInstance::CreateInterimBuilder` mentions some case. But that will likely need some special handling (because non-top-level QWidget will usually expect a QWidget parent). > It does... now I know where to start. I'll keep you up-to-date on my > progress. Great! > I had not anticipated this complication about the sidebar. I tried the > approach you mentioned (with the undocked sidebar) and got this: > > warn:vcl.qt:15757:15757:vcl/qt6/../qt5/QtBuilder.cxx:411: > QtBuilder::applyPackingProperties not yet implemented for this case > warn:vcl.qt:15757:15757:vcl/qt6/../qt5/QtBuilder.cxx:241: Widget type not > supported yet: GtkToolbar > soffice.bin: > /home/rafael/Programming/libreoffice/vcl/qt6/../qt5/QtBuilder.cxx:243: > QObject* QtBuilder::makeObject(QObject*, std::u16string_view, const > rtl::OUString&, BuilderBase::stringmap&): Assertion `false && "Widget type > not supported yet"' failed. > > I'm guessing GtkToolbar will have to be first. Yes. What I'd usually do in this case is look for a simpler dialog also using GtkToolbar (or alternatively temporarily remove some other non-supported widgets from the dialog) to be able to focus on (and test) one thing at a time, then come back to the original dialog and see what else is missing. I'll look a bit further into `QtInstanceTreeView` in the meantime, plan to at least upstream what I have on my current WIP branch, to avoid we duplicate work on that later on.
(In reply to Michael Weghorn from comment #5) > I'll look a bit further into `QtInstanceTreeView` in the meantime, plan to > at least upstream what I have on my current WIP branch, to avoid we > duplicate work on that later on. Changes pending in Gerrit now, see relation chain up to https://gerrit.libreoffice.org/c/core/+/177574