[GridDelegate] Fix gaps in corners of thumbnailArea highlight
ClosedPublic

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
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.Apr 30 2019, 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.EditedApr 30 2019, 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.EditedMay 11 2019, 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.EditedMay 14 2019, 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

ndavis updated this revision to Diff 61167.Jul 4 2019, 6:25 PM
  • Remove radius when thumbnail is present
ndavis added a comment.EditedJul 4 2019, 6:27 PM

Anyone want to approve or request changes? It's not perfect, but it's a simple improvement. The ideal solution would require me to work around a problem with QML where images can't have their corners rounded by a parent object's border radius.

filipf accepted this revision.Jul 4 2019, 8:22 PM
This revision is now accepted and ready to land.Jul 4 2019, 8:22 PM
This revision was automatically updated to reflect the committed changes.