search

Ranchero-Software / NetNewsWire

RSS reader for macOS and iOS.
https://netnewswire.com/
MIT License
8.4k stars 531 forks source link

iOS: improve colors management for pure white/black favicons #1731

Closed srhii closed 4 years ago

srhii commented 4 years ago

9B8447C6-81DC-4C85-8897-4C53F927C75B

vincode-io commented 4 years ago

@sergeykrasilnik Could you do me a favor and supply that feed link for the pure white icon you ran into?

vincode-io commented 4 years ago

We will want to assign this to Mac 5.1 when it is fixed for iOS 5.0.1.

srhii commented 4 years ago

http://feeds.feedburner.com/dcurtis

flowinho commented 4 years ago

Suggestion:

We can use Apples Accelerate framework to count color distribution in a fav-icon and when all values are close to white (nut pure white because very subtle greys will also be hard to see) we draw a 50% Grey (because that’s the standard for neutral colors) around it - but only in “light mode”, not in dark mode.

The same thing should be happening for almost pure black Favicons when the app is in Darkmode.

vincode-io commented 4 years ago

We do this already for dark mode. We just need to do it for light mode too.

flowinho commented 4 years ago

Ah! Nice! Where can I find the reference?

vincode-io commented 4 years ago

https://github.com/Ranchero-Software/NetNewsWire/blob/master/Shared/Extensions/IconImage.swift

flowinho commented 4 years ago

Perfect, I can work with that.

flowinho commented 4 years ago

Feeds to test pure black icons (to verify that old functionality is still working): https://500ish.com/feed

srhii commented 4 years ago

How about severe cases like this http://cherenkevich.com/feed/ pure black square icon that fills all the space?)

flowinho commented 4 years ago

@sergeykrasilnik That's really a hefty edge-case. Either we zoom the icon and display it, or we do nothing at all. The issue here at hand is how colours work: If we find out that the overall image is pure black, this does not mean we know it's content. A PNG with black pixels and some transparent background would still evaluate as pure black but should nevertheless gain some background color. We could examine the content of the feed-icon but that's a lot of engineering for a massive edge-cases of someone choosing a black square that fills all pixels as his feed icon.

srhii commented 4 years ago

I understand that and also that this is almost a single case problem but still an interesting design task)

36D81341-E10F-4151-86F2-3E60F1BDFF2D

Interesting that Reeder on top of adding extra white space also applies some kind of filter or something for dark icons so they aren’t pure black.

flowinho commented 4 years ago

That would mean messing with the icon of the original feed owner. I mean, that's totally possible but... I don't like it. It's not my decision, everything design-wise is the territory of @brentsimmons but I'd say we shouldn't do it.

srhii commented 4 years ago

Totally agree. Messing with colors not cool. Just an observation about Reeder.

vincode-io commented 4 years ago

I'm assigning this to Brent to determine if we need to do this for the Mac. Since we don't use pure black or white on the Mac, it might not be necessary.

flowinho commented 4 years ago

But it was already in place for pure black icons on the Mac. At least there was code.

vincode-io commented 4 years ago

It wasn't integrated with the Mac app yet.

flowinho commented 4 years ago

Ah okay.

vincode-io commented 4 years ago

I was mistaken, we do use pure white on the Mac for the timeline. I'm going to go ahead and reassign to myself to implement it.