Add Image resizeMode repeat on iOS #7968

janicduplessis · 2016-06-07T07:09:32Z

This adds a new resize mode for iOS 'repeat' that tiles the image over it's frame. This allow to easily create a view with a repeating background pattern which there is no way to do at the moment without including a bunch of different sized assets.

I'm not 100% sure it should be a resizeMode or a separate prop but I went with resizeMode since it made more sense to me and the are not really any use cases where we'd want to use this with another resizeMode other than 'stretch'.

Test plan

Tested mainly by adding a UIExplorer example, also tested that changing the resizeMode prop from and to 'repeat' worked properly.

I'd like to implement this on Android too but it is a bit trickier since Fresco's ImageView doesn't support image tiling and would require submitting a PR there too :(

ghost · 2016-06-07T07:10:39Z

By analyzing the blame information on this pull request, we identified @nicklockwood and @bnham to be potential reviewers.

janicduplessis · 2016-06-07T07:11:39Z

Libraries/Image/RCTImageUtils.m

@@ -64,6 +64,7 @@ CGRect RCTTargetRect(CGSize sourceSize, CGSize destSize,

  switch (resizeMode) {
    case RCTResizeModeStretch:
+    case RCTResizeModeRepeat:


I'm not 100% sure what these functions should return and what they are used for so maybe someone familiar with it could confirm that this is fine.

nicklockwood · 2016-06-10T10:15:34Z

Cc @dmmiller @vjeux - thoughts on whether we're happy with this approach for image tiling? We've gone back and forth on this a bit with respect to whether we should attempt to implement the CSS background-image spec for all Views (there's a PR for that already), or make tiling a feature of <Image> as Janic has done here.

My instinct is that this is probably the right way to go, even though it deviates from the web. I've no idea what would be involved in making it work on Android though.

mkonicek · 2016-06-14T21:14:51Z

Just asked in our internal Fresco group about tiling.

vjeux · 2016-06-14T21:24:53Z

As long as we can make it work on the web via some kind of polyfill, I'm happy with whatever we end up with.

mkonicek · 2016-06-14T21:31:34Z

I also like this but know little about the web :)

janicduplessis · 2016-06-14T21:46:20Z

@mkonicek Awesome, thanks!

@vjeux This implementation should be simple to polyfill on web via background-image + background-repeat: repeat + background-position: top left + background-size: <image-size>. It is pretty much a super simple version of what is doable with background images on web.

vjeux · 2016-06-14T22:01:12Z

👍

mkonicek · 2016-06-15T14:17:31Z

The main thing I'm concerned about is adding this on iOS only without knowing how to add it on Android. Let's see what the people working on Fresco say :)

janicduplessis · 2016-06-18T06:27:06Z

@mkonicek I managed to hack something together that works in fresco so it's definitely doable. Let me know if you get some news from the fresco team. I can always cleanup my implementation and send a PR to fresco.

mkonicek · 2016-06-20T13:13:39Z

@janicduplessis That's amazing! I've pinged them again. A PR for Fresco might be the right way to do it, let's wait for them to reply first to see if there's already an API for it.

javache · 2016-06-20T17:46:38Z

Libraries/Image/RCTImageUtils.m

+
+    case RCTResizeModeRepeat:
+
+      return false;


nit: return NO instead of false

javache · 2016-06-20T17:52:28Z

Looks good to me, just a couple of nits.

janicduplessis · 2016-06-21T16:55:24Z

@javache Updated!

javache · 2016-06-22T10:20:05Z

@facebook-github-bot shipit

ghost · 2016-06-22T10:20:40Z

Thanks for importing. If you are an FB employee go to Phabricator to review.

mkonicek · 2016-06-28T13:56:20Z

@janicduplessis I got a reply from the Fresco team, sorry about the delay:

Summary: This adds a new resize mode for iOS 'repeat' that tiles the image over it's frame. This allow to easily create a view with a repeating background pattern which there is no way to do at the moment without including a bunch of different sized assets. I'm not 100% sure it should be a resizeMode or a separate prop but I went with resizeMode since it made more sense to me and the are not really any use cases where we'd want to use this with another resizeMode other than 'stretch'. **Test plan** Tested mainly by adding a UIExplorer example, also tested that changing the resizeMode prop from and to 'repeat' worked properly. ![screen shot 2016-06-07 at 3 06 17 am](https://cloud.githubusercontent.com/assets/2677334/15848755/d95d8046-2c5c-11e6-9f3d-1ce8a1c9c846.png) I'd like to implement this on Android too but it is a bit trickier since Fresco's ImageView doesn't support image tiling and would require submitting a PR there too :( Closes facebook/react-native#7968 Differential Revision: D3469119 Pulled By: javache fbshipit-source-id: ab9dbfe448a5b0771dbf0c41fcceeb366210f583

Summary: This adds a new resize mode for iOS 'repeat' that tiles the image over it's frame. This allow to easily create a view with a repeating background pattern which there is no way to do at the moment without including a bunch of different sized assets. I'm not 100% sure it should be a resizeMode or a separate prop but I went with resizeMode since it made more sense to me and the are not really any use cases where we'd want to use this with another resizeMode other than 'stretch'. **Test plan** Tested mainly by adding a UIExplorer example, also tested that changing the resizeMode prop from and to 'repeat' worked properly. ![screen shot 2016-06-07 at 3 06 17 am](https://cloud.githubusercontent.com/assets/2677334/15848755/d95d8046-2c5c-11e6-9f3d-1ce8a1c9c846.png) I'd like to implement this on Android too but it is a bit trickier since Fresco's ImageView doesn't support image tiling and would require submitting a PR there too :( Closes facebook#7968 Differential Revision: D3469119 Pulled By: javache fbshipit-source-id: ab9dbfe448a5b0771dbf0c41fcceeb366210f583

SudoPlz · 2017-05-03T21:31:47Z

Any idea when the repeat param will be added to Android? It doesn't seem to be on the road map .

SudoPlz · 2017-06-13T21:12:07Z

Hey @janicduplessis would you be kind enough to share your code for fresco?

janicduplessis · 2017-06-19T23:33:19Z

@SudoPlz Sorry I can't find it anymore. There isn't any plans to implement it for now so the best way would be for someone from the community to contribute support for it in Fresco then we could use it here.

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. #14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in #17398 (comment), I have neither updated nor tested the "Flat" UI implementation - everything compiles but I've taken [this comment](#12770 (comment)) to mean there's no point in trying to wade through it on my own right now; I'm happy to tackle it if given some pointers. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes #17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (facebook#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. facebook#14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in facebook#17398 (comment), I have neither updated nor tested the "Flat" UI implementation - everything compiles but I've taken [this comment](facebook#12770 (comment)) to mean there's no point in trying to wade through it on my own right now; I'm happy to tackle it if given some pointers. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes facebook#17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (facebook#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. facebook#14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in facebook#17398 (comment), I have neither updated nor tested the "Flat" UI implementation - everything compiles but I've taken [this comment](facebook#12770 (comment)) to mean there's no point in trying to wade through it on my own right now; I'm happy to tackle it if given some pointers. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes facebook#17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249 (cherry picked from commit 0459e4f)

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (facebook#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. facebook#14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in facebook#17398 (comment), I have neither updated nor tested the "Flat" UI implementation - everything compiles but I've taken [this comment](facebook#12770 (comment)) to mean there's no point in trying to wade through it on my own right now; I'm happy to tackle it if given some pointers. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes facebook#17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249

ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jun 7, 2016

janicduplessis reviewed Jun 7, 2016
View reviewed changes

janicduplessis changed the title ~~Add Image resizeMode repeat~~ Add Image resizeMode repeat on iOS Jun 7, 2016

Add Image resizeMode repeat

9b133ab

janicduplessis force-pushed the image-repeat branch from 3a9b355 to 9b133ab Compare June 18, 2016 06:24

javache reviewed Jun 20, 2016
View reviewed changes

Libraries/Image/RCTImageUtils.m

case RCTResizeModeRepeat:

return false;

Copy link

Member

javache Jun 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: return NO instead of false

Fix nits

dcfe43d

ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jun 22, 2016

ghost closed this in 4c83237 Jun 22, 2016

motiz88 mentioned this pull request Dec 31, 2017

Support Image resizeMode=repeat on Android #17404

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Image resizeMode repeat on iOS #7968

Add Image resizeMode repeat on iOS #7968

janicduplessis commented Jun 7, 2016 •

edited

Loading

ghost commented Jun 7, 2016

janicduplessis Jun 7, 2016

nicklockwood commented Jun 10, 2016

mkonicek commented Jun 14, 2016 •

edited

Loading

vjeux commented Jun 14, 2016

mkonicek commented Jun 14, 2016

janicduplessis commented Jun 14, 2016

vjeux commented Jun 14, 2016

mkonicek commented Jun 15, 2016 •

edited

Loading

janicduplessis commented Jun 18, 2016

mkonicek commented Jun 20, 2016

javache Jun 20, 2016

javache commented Jun 20, 2016

janicduplessis commented Jun 21, 2016

javache commented Jun 22, 2016

ghost commented Jun 22, 2016

mkonicek commented Jun 28, 2016 •

edited

Loading

SudoPlz commented May 3, 2017 •

edited

Loading

SudoPlz commented Jun 13, 2017 •

edited

Loading

janicduplessis commented Jun 19, 2017

Add Image resizeMode repeat on iOS #7968

Add Image resizeMode repeat on iOS #7968

Conversation

janicduplessis commented Jun 7, 2016 • edited Loading

ghost commented Jun 7, 2016

janicduplessis Jun 7, 2016

Choose a reason for hiding this comment

nicklockwood commented Jun 10, 2016

mkonicek commented Jun 14, 2016 • edited Loading

vjeux commented Jun 14, 2016

mkonicek commented Jun 14, 2016

janicduplessis commented Jun 14, 2016

vjeux commented Jun 14, 2016

mkonicek commented Jun 15, 2016 • edited Loading

janicduplessis commented Jun 18, 2016

mkonicek commented Jun 20, 2016

javache Jun 20, 2016

Choose a reason for hiding this comment

javache commented Jun 20, 2016

janicduplessis commented Jun 21, 2016

javache commented Jun 22, 2016

ghost commented Jun 22, 2016

mkonicek commented Jun 28, 2016 • edited Loading

SudoPlz commented May 3, 2017 • edited Loading

SudoPlz commented Jun 13, 2017 • edited Loading

janicduplessis commented Jun 19, 2017

janicduplessis commented Jun 7, 2016 •

edited

Loading

mkonicek commented Jun 14, 2016 •

edited

Loading

mkonicek commented Jun 15, 2016 •

edited

Loading

mkonicek commented Jun 28, 2016 •

edited

Loading

SudoPlz commented May 3, 2017 •

edited

Loading

SudoPlz commented Jun 13, 2017 •

edited

Loading