[GridDelegate] Fix gaps in corners of thumbnailArea highlight
Needs ReviewPublic

Authored by ndavis on Mar 31 2019, 4:09 AM.

Details

Reviewers
None
Group Reviewers
Plasma
VDG
Summary

There was a small gap in the corners of the thumbnailArea highlight rectangle that was noticable on dark thumbnails.

Test Plan

Before:



After:

Diff Detail

Repository
R296 KDeclarative
Branch
grid-delegate-rectangle (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10288
Build 10306: arc lint + arc unit
ndavis created this revision.Mar 31 2019, 4:09 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 31 2019, 4:09 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ndavis requested review of this revision.Mar 31 2019, 4:09 AM
ndavis edited the test plan for this revision. (Show Details)Mar 31 2019, 4:09 AM
ndavis updated this revision to Diff 55106.EditedMar 31 2019, 4:30 AM
  • Remove radius from thumbnailArea highlight

Having the same radius as the thumbnailArea allowed it to fit perfectly when the thumbnails weren't filling the corners of the thumbnailArea, but it still left a small gap on thumbnails that completely filled the thumbnailArea. Removing the radius makes the highlight slightly overlap with the inner corners of the thumbnail outline, but it's not really noticeable.

ndavis edited the test plan for this revision. (Show Details)Mar 31 2019, 4:33 AM

It's probably going to be more noticeable for High DPI users. It seems like there must be a way to fix this rather than just removing the thumbnail radius.

It's probably going to be more noticeable for High DPI users. It seems like there must be a way to fix this rather than just removing the thumbnail radius.

I could remove the radius for the thumbnailArea as well. Here's how the current change looks at 2x scaling:

Here is how no radius for the thumbnailArea and thumbnail highlight looks:

Another alternative would be to turn the thumbnail outline into a real outline instead of a rounded rectangle behind the thumbnailArea. Then it could go on top of the thumbnailArea and the thumbnail highlight could fit perfectly inside of it. This would give previews of rectangular images rounded corners and is significantly more complicated than the current change, but I feel like it should be possible.

In D20140#450349, @GB_2 wrote:

Ping

Sorry, I'm just busy right now and I'm not that familiar with QML, so it won't be quick and easy for me.

filipf added a subscriber: filipf.Tue, Apr 30, 1:58 AM

You can fix the rounding bleeding into image previews by doing the following at line 116:

radius: thumbnailAvailable ? 0 : Kirigami.Units.smallSpacing

filipf added a comment.EditedTue, Apr 30, 2:08 AM

Which still leaves you with fixing the gaps when there's not thumbnail. I'd suggest to do:

radius: thumbnailAvailable ? 0 : Kirigami.Units.smallSpacing / 2

and call it a day :P

ndavis added a comment.EditedSat, May 11, 3:13 PM

Which still leaves you with fixing the gaps when there's not thumbnail. I'd suggest to do:

radius: thumbnailAvailable ? 0 : Kirigami.Units.smallSpacing / 2

and call it a day :P

Unfortuantely, that still leaves small white corners on the thumbnails that don't take up the entire area.

Which still leaves you with fixing the gaps when there's not thumbnail. I'd suggest to do:

radius: thumbnailAvailable ? 0 : Kirigami.Units.smallSpacing / 2

and call it a day :P

Unfortuantely, that still leaves small white corners on the thumbnails that don't take up the entire area.

In practice, the thumbnail delegates are much smaller; is anyone gonna notice that (famous last words)?

ndavis added a comment.EditedTue, May 14, 6:20 AM

In practice, the thumbnail delegates are much smaller; is anyone gonna notice that (famous last words)?

If you and anyone else reviewing this patch thinks it's OK, I'll just remove the radius for simplicity and consistency, but you seemed like you didn't want small bits of whiteness in the corners:

It's probably going to be more noticeable for High DPI users. It seems like there must be a way to fix this rather than just removing the thumbnail radius.

BTW, that last screenshot you quoted has 3x scaling