[KDE Usability] Review Request 108504: [High-dpi issues] Fix KIconDialog

[Kai Uwe Broulik]

> On Jan. 20, 2013, 2:40 p.m., Thomas Lübking wrote:
> > kio/kfile/kicondialog.cpp, line 141
> > <http://git.reviewboard.kde.org/r/108504/diff/2/?file=108215#file108215line141>
> >
> >     Would it for such static sizes (why 60px in the first place?) not be more "correct" to use sh. like 60*100*2/(widget->physicalDpiX()+widget->physicalDpiY()) where "100" wold be the "standard" dpi? (could also be eg. 72 or 96)
> >     
> >     Right now (in latter RRs) you assign various configurable icon sizes to "remotely" related views.
> >     That might be correct and intended, but it's also a behavioral change and this time, desktop icons /can/ be assigned to 256px through the GUI - even on a 72dpi screen - and that's unlikely the intended size here.

>From what I can tell all the iconButtons are using Desktop icon sizes (either properly or some constant), so using it there which is triggered by the iconButton made sense to me.
And since KDE is nowhere near dpi independent, is the easiest and best way to do this imho. Otherwise we should just patch KIcon to return an icon size based on the screen dpi … but we don't use SVG icons and only have a limited amount of icon sizes (there is for example no 96x96 which is double 48x48 icon), so I see no other way.


- Kai Uwe


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


On Jan. 20, 2013, 11:15 a.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108504/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2013, 11:15 a.m.)
> 
> 
> Review request for kdelibs and KDE Usability.
> 
> 
> Description
> -------
> 
> This makes the KIconDialog (the dialog where you can choose icons for eg. folders) respect the global icon size. Almost all sizes were hardcoded but the patch does away with all of this and works fine with all icon sizes and big font sizes. Also made it aware of FontMetrics (atm with bigger fonts, they also get clipped) and adjusts the grid height accordingly.
> 
> Was fun diving into that "ancient" code :)
> 
> 
> Diffs
> -----
> 
>   kio/kfile/kicondialog.cpp b7d646f 
> 
> Diff: http://git.reviewboard.kde.org/r/108504/diff/
> 
> 
> Testing
> -------
> 
> Yup, see screenshot.
> 
> The only issue that remains is the initial size of the dialog to make it show 4 rows of icons. In the current implementation it just adds another 100px to the dialog height (cf. line 490), which is easy, if all the sizes are known and fixed, but with variing sizes this becomes an issue and I could not think of a proper solution. I probably need to add a sizeHint (tried in the private class, didn't help there)? The easiest but not neccessarily best solution would be to just set the minimumHeight to 4 rows and done.
> 
> 
> File Attachments
> ----------------
> 
> Icon Dialog with 200dpi
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/01/20/icondialog.png
> Icon Dialog with 200dpi (without patch)
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/01/20/icondialog2.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130120/f40dd69d/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-usability mailing list
kde-usability at kde.org
https://mail.kde.org/mailman/listinfo/kde-usability