Bug 386076 - QToolButton is drawn incorectly when there are extra size constraints
Summary: QToolButton is drawn incorectly when there are extra size constraints
Status: RESOLVED INTENTIONAL
Alias: None
Product: Breeze
Classification: Plasma
Component: QStyle (show other bugs)
Version: 5.10.4
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Hugo Pereira Da Costa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-22 16:11 UTC by Michał D. (Emdek)
Modified: 2017-10-31 11:13 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michał D. (Emdek) 2017-10-22 16:11:12 UTC
There exist drawing issues when QToolButton has size constraint that prevents it from using preferred size. 
Fusion only applies clipping while Breeze and Oxygen also draw icon in wrong place.

It could be also considered if the style should try to fit text (using at least part of space wasted directly under icon) or maybe even skip it when available space allows to fit less than half height (or other factor) of the text (it's probably better than clipping it, since it's unreadable anyway).

Example code, compare results when running using Fusion style or Breeze or Oxygen.

	#include <QtWidgets/QApplication>
	#include <QtWidgets/QBoxLayout>
	#include <QtWidgets/QToolButton>
	
	int main(int argc, char *argv[])
	{
		QApplication application(argc, argv);
		QWidget widget;
		QToolButton normalToolButton(&widget);
		normalToolButton.setIcon(QIcon::fromTheme("text-html"));
		normalToolButton.setIconSize({64, 64});
		normalToolButton.setText("Some Text");
		normalToolButton.setToolButtonStyle(Qt::ToolButtonTextUnderIcon);
	
		QToolButton constrainedToolButton(&widget);
		constrainedToolButton.setIcon(QIcon::fromTheme("text-html"));
		constrainedToolButton.setIconSize({64, 64});
		constrainedToolButton.setText("Some Text");
		constrainedToolButton.setToolButtonStyle(Qt::ToolButtonTextUnderIcon);
		constrainedToolButton.setFixedSize({64, 72});
	
		QBoxLayout layout(QBoxLayout::TopToBottom, &widget);
		layout.addWidget(&normalToolButton);
		layout.addWidget(&constrainedToolButton);
	
		widget.show();
	
		return application.exec();
	}
Comment 1 Hugo Pereira Da Costa 2017-10-22 16:17:31 UTC
Hi, 
thanks for reporting the issue, but ...

if you apply fixed size to widgets, disregarding the one recommanded by the style, you're on your own. There is now way the style can work in all cases when used in an incomplete manner, or overconstrained, or inconsistent. 
If you want overconstrained widgets to work in all cases you'll have to write the paintEvent yourself too. 

There is most likely a way to make your particular example work with breeze/oxygen but then it will likely
- break in other places
- not work for other cases of specialization.

Wontfix, sorry

Hugo
Comment 2 Michał D. (Emdek) 2017-10-22 16:19:50 UTC
Hello
I agree, but logic that aligns icon should be fixed, drawing it outside of frame, above proper location, is simply weird.
Comment 3 Hugo Pereira Da Costa 2017-10-22 16:36:03 UTC
As far as I know there is nothing incorrect about the placement of the text and icons in the example you sent: icon is cropped at the top, text at the bottom, and in fact by the same amount. 
The logic behind it is that the text and the icon are placed in a rect that accomodate their minimum size and some style defined vertical spacing between the two, then this rect is centered on the actual toolbutton rect. 
This is what leads to the result you see.
Comment 4 Michał D. (Emdek) 2017-10-22 16:38:13 UTC
I guess that Fusion should be considered as reference implementation, so perhaps it's better to be consistent with it and at least align it to the top and only cut off bottom part, at least it looks "less broken" that way.
Comment 5 Hugo Pereira Da Costa 2017-10-22 16:51:16 UTC
I disagree.
Say someone wants to trim the margins by just the right amount that both the text and the icon still apear in the trimmed button, then for fusion he/she will complain that the icon and text are not centered anymore and that margins are different between the top and the bottom. 
There is no solution to this problem, because this is not a well-formed problem. 
And fusion is not a reference implementation. It has bugs that other styles dont (though I can't remember of one on top of my head right now).
Comment 6 Michał D. (Emdek) 2017-10-24 07:51:09 UTC
Well, for me it's the closest thing to a reference (it's official fallback after all, so it's one of the most used styles, especially after this change: https://0x0.st/F2Y.txt). ;-)

Might be not perfect but it's in pretty good shape, especially when comparing to platform style for macOS, it's both buggy and partially outdated (tab bar in document mode doesn't look at all like these used by Finder or Safari)...
Comment 7 Michał D. (Emdek) 2017-10-24 07:59:35 UTC
I've got second thoughts on this, I think that I've should report it to BQI instead, since it's more like inconsistency here:
http://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/widgets/qtoolbutton.cpp#n238

To be exact, I mean the part where text is not elided when width is constrained (and still bigger than icon size), for example QTabBar elides text on widget level, but it's missing in QToolButton.
Comment 8 Michał D. (Emdek) 2017-10-31 11:13:23 UTC
Reported upstream:
https://bugreports.qt.io/browse/QTBUG-64132