Review Request 111608: Fix "truncated Details View header" when using a non-Oxygen style

[Christoph Feck]

> On July 21, 2013, 8:53 p.m., Christoph Feck wrote:
> > For whatever reason, it does not affect Skulpture style. You sure can commit it, while I try to find out if the issue is with Skulpture or the header painting.
> 
> Frank Reininghaus wrote:
>     Thanks for testing! I tested only Oxygen and Plastique and hoped that the change would also fix the problems with other styles. But if I understand correctly, it does not make things worse, so I'll go ahead and commit it.
>     
>     If you find out that the root cause of the problem with Skulpture is on the Dolphin side, just let us know!
> 
> Christoph Feck wrote:
>     Just had a look at this issue.
>     
>     First, I found out why it previously did not work: The CE_HeaderEmptyArea rendering was simply clipped away!
>     
>     Inserting a painter->setClipRect(opt.rect) in front of drawControl(QStyle::CE_HeaderEmptyArea) made the empty area appear correctly with Skulpture and Plastique. It probably only worked with Oxygen, because it had its own call of setClipRect().
>     
>     The way you are now rendering the empty area is not correct. You effectively render an additional section. If you want to keep that, please at least set the QStyle::State_Enabled in option.state. That is the reason it looks wrong with Skulpture.
>     
>     Otherwise, I would suggest to revert the commit, and insert the setClipRect() call.
> 
> Frank Reininghaus wrote:
>     Thanks for the analysis, Christoph! Maybe we should just add the setClipRect() call in KDE/4.11 and do the revert+setClipRect() in master.
>     
>     BTW, why is the setClipRect() call only needed for the "empty area"? Should we better add that call also to the code that paints the 'real' headers?

My previous comment probably was a bit confusing, let me simplify.

What you can do is:
1. keep "render a section instead of empty area"
2. revert the commit

If you do 1. then please add State_Enabled.
If you do 2. then you have to insert the clipping call to fix the original bug.

If you do not revert, there is no need to insert the clipping call, because rendering a section automatically clips. This is also the reason you have to re-set the clip rect when later rendering the empty area. Probably for performance reasons, the clip region is not restored between the sections.


- Christoph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111608/#review36269
-----------------------------------------------------------


On July 22, 2013, 5:11 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111608/
> -----------------------------------------------------------
> 
> (Updated July 22, 2013, 5:11 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> When using a non-Oxygen style, and the widths of the columns in Details View are too small to fill the entire width of the view, the headers look "truncated" (see screen shot in the bug report). This does not happen with Oxygen - an empty header is shown instead of the white empty space.
> 
> I looked at the code and found that the painting of the "empty header" is inconsistent with the other headers, which is probably the reason why the other styles get confused:
> 
> a) No QStyleOptionHeader is used
> b) Even if an empty header must be drawn, the last column is drawn with the option QStyleOptionHeader::End.
> 
> I propose to change this by moving the code for painting the empty header to KItemListHeaderWidget::paintRole(): when the last column is drawn, it checks if there is some space left that needs to be filled and uses QStyleOptionHeader::Middle for the last column then (I don't see another way how issue b) could be solved cleanly).
> 
> 
> This addresses bug 301800.
>     http://bugs.kde.org/show_bug.cgi?id=301800
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/private/kitemlistheaderwidget.cpp 0f1f20b 
> 
> Diff: http://git.reviewboard.kde.org/r/111608/diff/
> 
> 
> Testing
> -------
> 
> Works for me with Oxygen and Plastique.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130816/d9d051e8/attachment.htm>