Comments (7)
About compound filter identifiers, we could inner filter identifiers separated with _
.
Of course, the order of inner filters is very important to produce the same compound identifier.
from alamofireimage.
Honestly I think this is a GREAT idea! Do you want to put a PR together with this functionality or would you like me to do it? If you do end up putting it together...I have a few suggestions:
- Let's create a
CompositeImageFilter
protocol to enforce thefilters
property and theImageFilter
conformance. - Would be slick to use a protocol extension to create the
identifier
. Definitely needs to be joined by either a-
or_
in the correct order that the filters are added. - We should rework all the
Multi-Pass
image filters to beCompositeImageFilter
s instead. - Let's definitely gets tests to cover this addition.
Like I said, if you want to roll this yourself that would be awesome. Otherwise I'll implement this probably next week.
Thanks! 🍻
from alamofireimage.
Cool! Yes, I'll try to implement it this week-end and PR by next sunday.
About you suggestions:
- Of course, always protocol first.
CompositeImageFilter
will only add avar filters: [ImageFilter] { get }
property. - Agreed, composite filters should only be worried about providing the
filters
in the correct order. The protocol extension will provide a default implementation foridentifier
andfilter
. - Yes, I think multi-pass filters will be much lighter (less code) this way :-)
- I'll do my best for tests coverage, based on the existing tests.
When you'll get the PR, feel free to accommodate the code to your standards. 👌 👍
from alamofireimage.
Several remarks:
- When converting filters such as
AspectScaledToFillSizeWithRoundedCornersFilter
, the image comparison test fails because of the radius not being scaled. In the current version ofAspectScaledToFillSizeWithRoundedCornersFilter
, I see the radius is multiplied by the input image scale. Shouldn't this be the responsibility ofRoundedCornersFilter
which would automatically apply the scale based on the input image? I don't understand why multi-pass filters apply a scale when usingaf_imageWithRoundedCornerRadius
andRoundedCornersFilter
does not. - Should current multi-pass filters still conform to
Sizable
andRoundable
? Especially,AspectScaledToFillSizeCircleFilter
andScaledToSizeCircleFilter
should not be consideredRoundable
anymore. It was interesting to get an automatic identifier based on size and corner radius, but now the identifier will be made of composite filter identifiers. - Multi-pass filters converted to
CompositeImageFilter
will not get the same name as before. Instead ofScaledToSizeWithRoundedCornersFilter-size:(200x100)-radius:(20)
, we'll getScaledToSizeFilter-size:(200x100)_RoundedCornersFilter-radius:(20)
. Is this ok for you?
from alamofireimage.
Hi @damienrambout,
- The images should look exactly the same between all screen scales. If you don't apply adjust the radius by the scale factor, then the rounded corner is much different on all screen scales. This is exactly why I created Multi-Pass filters the way I did. You'll need to keep this functionality. If it ends up feeling like a hack to add a flag to the
RoundedCornersFilter
for making all rounded corners the same across all screen scales, then I'd say theCompositeImageFilter
protocol may not be a good idea after all. - Nope.
- Fine by me.
from alamofireimage.
Hi @cnoon,
Of course all images should look the same between screen scales. This is why I wanted to apply the image scale in the RoundedCornersFilter
, which I believe is the correct way to do it. If I do that, multi-pass filter tests will pass (since it will be exactly like you implemented these filters in the first place), but then testThatRoundedCornersFilterReturnsCorrectFilteredImage
will not pass on a screen-scale > 1.
In this test, the expected image has no scale specified (pirate-radius-20.png
) but it is still loaded using the current screen scale. Shouldn't this kind of images (no scale specified) always be considered like @1x
, like ones downloaded from the internet?
If pirate-radius-20.png
is loaded using UIImage(named:)
, we get a an image with a size of (350.0, 210.0)
and a scale of 1.0
.
from alamofireimage.
I'm working on this issue now @damienrambout. There's definitely a solution. Let's redirect all future comments to #8. I'm going to close this issue out.
from alamofireimage.
Related Issues (20)
- Useless concurrent attribute for synchronizationQueue in AutoPurgingImageCache HOT 1
- Cannot find type 'ImageDownloader' in scope HOT 1
- how can I achieve an AutoPurgingImageCache with unlimited size? HOT 1
- Fix Package.swift and release 3.5.3 for older version use via SPM HOT 1
- Always downloading images in 2000, 1000 as max size HOT 5
- Can't resolve dependencies starting on version 4.0.0 HOT 1
- unpredictable crash HOT 1
- Response Content-Type "text/plain" does not match any acceptable types HOT 1
- I
- Image is not cashing if we download image using ImageDownloader and restart the application HOT 2
- How do you cancel all pending and current image download requests in AlamofireImage? (Possible Feature Request) HOT 1
- Feature request: Support `image/avif` to Accept header for iOS 16, macOS Ventura and more. HOT 5
- App crash on ios 11.0.2, xcode 14.0.1 HOT 1
- Does AlamofireImage support resizing animated GIF? HOT 1
- Incorrect associatedObject use generates many Xcode 15 warnings HOT 2
- Is Alamofire image vulnerable to CVE-2023-4863 HOT 1
- ImageDownloader.download [EXC_BAD_ACCESS KERN_INVALID_ADDRESS]
- Apple Privacy Manifest HOT 10
- Customize interceptor on ImageDownloader's Session HOT 1
- Request for updated iOS SDK version with the xcprivacy manifest HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from alamofireimage.