Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add bounds to raster source #3672

Merged
merged 2 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,8 @@ class RNMBXRasterSource(context: Context?) : RNMBXTileSource<RasterSource?>(cont
companion object {
const val DEFAULT_TILE_SIZE = 512
}
}

fun setSourceBounds(value: Array<Double>) {
bounds = value
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import com.facebook.react.viewmanagers.RNMBXRasterSourceManagerInterface
import com.rnmapbox.rnmbx.events.constants.EventKeys
import com.rnmapbox.rnmbx.events.constants.eventMapOf
import javax.annotation.Nonnull
import com.facebook.react.bridge.ReadableArray
import com.rnmapbox.rnmbx.components.styles.RNMBXStyleImportManager
import com.rnmapbox.rnmbx.components.styles.RNMBXStyleImportManager.Companion
import com.rnmapbox.rnmbx.utils.Logger

class RNMBXRasterSourceManager(reactApplicationContext: ReactApplicationContext) :
RNMBXTileSourceManager<RNMBXRasterSource>(reactApplicationContext),
Expand Down Expand Up @@ -42,4 +46,36 @@ class RNMBXRasterSourceManager(reactApplicationContext: ReactApplicationContext)
override fun setExisting(source: RNMBXRasterSource, value: Dynamic) {
source.mExisting = value.asBoolean()
}
}

@ReactProp(name = "sourceBounds")
override fun setSourceBounds(source: RNMBXRasterSource, value: Dynamic) {
if (value.type.name == "Array") {
val readableArray: ReadableArray = value.asArray()

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check necessary here? We'll just silently error if the size is not 4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this, you will get an error like this:
Screenshot_1730282753

Personally, I prefer a more direct message, but I leave it up to you.

Copy link
Contributor

@mfazekas mfazekas Oct 30, 2024

Choose a reason for hiding this comment

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

Sorry, mybad, you're right I've missed the Logger.e at the end.

Nitpick: Can please you rewrite the logic so we have early return on error statements, and not, the success case

Something like:

if (value.type.name != "Array" || readableArray.size() != 4) {
   Logger.e(RNMBXRasterSourceManager.REACT_CLASS, "source bounds must be an array with left, bottom, top, and right values")
   return
}

val bboxArray = Array(4) { i -> readableArray.getDouble(i)
source.setSourceBounds(bboxArray)
...

if (readableArray.size() == 4) {
val bboxArray = Array(4) { i -> readableArray.getDouble(i) }

if(this.validateBbox(bboxArray)){
source.setSourceBounds(bboxArray)
} else {
Logger.e(RNMBXRasterSourceManager.REACT_CLASS, "source bounds contain invalid bbox")
}

return
}
}
Logger.e(RNMBXRasterSourceManager.REACT_CLASS, "source bounds must be an array with left, bottom, top, and right values")
}

fun validateBbox(bbox: Array<Double>): Boolean {
if (bbox.size != 4) return false

val (swLng, swLat, neLng, neLat) = bbox

val isLngValid = swLng in -180.0..180.0 && neLng in -180.0..180.0
val isLatValid = swLat in -90.0..90.0 && neLat in -90.0..90.0
val isSouthWestOfNorthEast = swLng < neLng && swLat < neLat

return isLngValid && isLatValid && isSouthWestOfNorthEast
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ abstract class RNMBXTileSource<T : Source?>(context: Context?) : RNMBXSource<T>(
var attribution: String? = null
var minZoomLevel: Int? = null
var maxZoomLevel: Int? = null
var bounds: Array<Double>? = null
var tMS = false
fun buildTileset(): TileSet {
val tileUrlTemplates =
Expand All @@ -35,10 +36,14 @@ abstract class RNMBXTileSource<T : Source?>(context: Context?) : RNMBXSource<T>(
if (attribution != null) {
builder.attribution(attribution)
}
if(bounds != null) {
val boundsArray = bounds!!.clone()
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need !! we've tested above for null already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this, you will get an error:

Smart cast to 'Array<Double>' is impossible, because 'bounds' is a mutable property that could have been changed by this time FAILURE: Build failed with an exception.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to do something like this:

bounds?.let { 
   builder.bounds(Arrays.asList(it))
}

builder.bounds(Arrays.asList(*boundsArray))
}
return builder.build()
}

companion object {
const val TILE_SPEC_VERSION = "2.1.0"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ public void setProperty(T view, String propName, @Nullable Object value) {
case "attribution":
mViewManager.setAttribution(view, new DynamicFromObject(value));
break;
case "sourceBounds":
mViewManager.setSourceBounds(view, new DynamicFromObject(value));
break;
default:
super.setProperty(view, propName, value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ public interface RNMBXRasterSourceManagerInterface<T extends View> {
void setTileSize(T view, Dynamic value);
void setTms(T view, Dynamic value);
void setAttribution(T view, Dynamic value);
void setSourceBounds(T view, Dynamic value);
}
11 changes: 11 additions & 0 deletions docs/RasterSource.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ FIX ME NO DESCRIPTION



### sourceBounds

```tsx
Array
```
An array containing the longitude and latitude of the southwest and northeast corners of
the source's bounding box in the following order: `[sw.lng, sw.lat, ne.lng, ne.lat]`.
When this property is included in a source, no tiles outside of the given bounds are requested by Mapbox GL.






Expand Down
7 changes: 7 additions & 0 deletions docs/docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -5052,6 +5052,13 @@
"type": "React.ReactElement \\| React.ReactElement[]",
"default": "none",
"description": "FIX ME NO DESCRIPTION"
},
{
"name": "sourceBounds",
"required": false,
"type": "Array",
"default": "none",
"description": "An array containing the longitude and latitude of the southwest and northeast corners of\nthe source's bounding box in the following order: `[sw.lng, sw.lat, ne.lng, ne.lat]`.\nWhen this property is included in a source, no tiles outside of the given bounds are requested by Mapbox GL."
}
],
"fileNameWithExt": "RasterSource.tsx",
Expand Down
4 changes: 4 additions & 0 deletions example/src/examples/FillRasterLayer/RasterSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ export default function RasterSourceExample() {
<RasterSource
id="stamen-watercolor"
tileSize={256}
sourceBounds={[
-74.01010105570786, 40.7096750598196, -74.00028742807824,
40.71670107507063,
]}
tileUrlTemplates={['https://tile.openstreetmap.org/{z}/{x}/{y}.png']}
/>
<RasterLayer
Expand Down
26 changes: 16 additions & 10 deletions ios/RNMBX/RNMBXRasterSource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,19 @@ public class RNMBXRasterSource : RNMBXSource {
typealias SourceType = RasterSource

@objc public var url: String? = nil

@objc public var tileUrlTemplates: [String]? = nil

@objc public var minZoomLevel: NSNumber?
@objc public var maxZoomLevel: NSNumber?
@objc public var tileSize: NSNumber?

@objc public var tms: Bool = false

@objc public var attribution: String?

@objc public var sourceBounds: [NSNumber]? = nil

override func makeSource() -> Source
{
#if RNMBX_11
Expand All @@ -28,27 +30,31 @@ public class RNMBXRasterSource : RNMBXSource {
} else {
result.tiles = tileUrlTemplates
}

if let tileSize = tileSize {
result.tileSize = tileSize.doubleValue
}

if let minZoomLevel = minZoomLevel {
result.minzoom = minZoomLevel.doubleValue
}

if let maxZoomLevel = maxZoomLevel {
result.maxzoom = maxZoomLevel.doubleValue
}

if tms {
result.scheme = .tms
}

if let attribution = attribution {
result.attribution = attribution
}


if let bounds = sourceBounds {
result.bounds = bounds.map { $0.doubleValue }
}

return result
}

Expand Down
8 changes: 6 additions & 2 deletions ios/RNMBX/RNMBXRasterSourceComponentView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ + (ComponentDescriptorProvider)componentDescriptorProvider

- (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &)oldProps
{
const auto &newProps = static_cast<const RNMBXRasterSourceProps &>(*props);
const auto &newProps = static_cast<const RNMBXRasterSourceProps &>(*props);
id idx = RNMBXConvertFollyDynamicToId(newProps.id);
if (idx != nil) {
_view.id = idx;
Expand Down Expand Up @@ -113,7 +113,11 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
if (attribution != nil) {
_view.attribution = attribution;
}

id sourceBounds = RNMBXConvertFollyDynamicToId(newProps.sourceBounds);
if (sourceBounds != nil) {
_view.sourceBounds = sourceBounds;
}

[super updateProps:props oldProps:oldProps];
}

Expand Down
2 changes: 1 addition & 1 deletion ios/RNMBX/RNMBXRasterSourceViewManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ @interface RCT_EXTERN_REMAP_MODULE(RNMBXRasterSource, RNMBXRasterSourceViewManag

RCT_EXPORT_VIEW_PROPERTY(tms, BOOL)
RCT_EXPORT_VIEW_PROPERTY(attribution, NSString)

RCT_EXPORT_VIEW_PROPERTY(sourceBounds, NSArray)

@end
6 changes: 6 additions & 0 deletions src/components/RasterSource.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ type Props = BaseProps & {
attribution?: string;

children?: React.ReactElement | React.ReactElement[];
/**
* An array containing the longitude and latitude of the southwest and northeast corners of
* the source's bounding box in the following order: `[sw.lng, sw.lat, ne.lng, ne.lat]`.
* When this property is included in a source, no tiles outside of the given bounds are requested by Mapbox GL.
*/
sourceBounds?: number[];
};

type NativeProps = Props;
Expand Down
1 change: 1 addition & 0 deletions src/specs/RNMBXRasterSourceNativeComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface NativeProps extends ViewProps {
tileSize: UnsafeMixed<Double>;
tms: UnsafeMixed<boolean>;
attribution: UnsafeMixed<string>;
sourceBounds: UnsafeMixed<Array<number>>;
}

export default codegenNativeComponent<NativeProps>(
Expand Down
Loading