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

CustomLayer component #41

Merged
merged 2 commits into from
Nov 29, 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
3 changes: 2 additions & 1 deletion src/content/examples/Index.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
'/examples/geolocate': 'Locate the User',
'/examples/image-loader': 'Load Images from URLs',
'/examples/custom-control': 'Custom Control',
'/examples/custom-protocol': 'Custom Protocols'
'/examples/custom-protocol': 'Custom Protocols',
'/examples/custom-layer': 'Custom Layer'
}
},
{
Expand Down
7 changes: 4 additions & 3 deletions src/content/examples/custom-control/content.svelte.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ description: Custom Control allows to easily create user defined controls.
---

<script lang="ts">
import CustomControl from "./CustomControl.svelte";
import Demo from "./CustomControl.svelte";
import demoRaw from "./CustomControl.svelte?raw";
import CodeBlock from "../../CodeBlock.svelte";
let { shiki } = $props();
</script>

<CustomControl />
<Demo />

<CodeBlock content={demoRaw} />
<CodeBlock content={demoRaw} {shiki} />
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Component renaming is correct, but shiki prop syntax needs standardization

The verification shows:

  • The CustomControl component is correctly used within its own example directory (src/content/examples/custom-control/)
  • The shiki prop is used consistently across all example files, but with two different syntaxes:
    • Most files use shiki={shiki}
    • Only custom-protocol and custom-control use the shorthand {shiki}
🔗 Analysis chain

Verify consistent component naming across examples

The component renaming and shiki prop addition appear correct. Let's verify this pattern is consistently applied across other example files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistency in component naming and shiki prop usage across example files

# Check for any remaining CustomControl references
echo "Checking for remaining CustomControl references:"
rg "CustomControl" "src/content/examples"

# Check for consistent CodeBlock shiki prop usage
echo "Checking CodeBlock shiki prop usage:"
rg "<CodeBlock" "src/content/examples"

Length of output: 3227

89 changes: 89 additions & 0 deletions src/content/examples/custom-layer/CustomLayer.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<script lang="ts">
import { MapLibre, CustomLayer } from 'svelte-maplibre-gl';
import maplibregl from 'maplibre-gl';

class CustomLayerImpl implements Omit<maplibregl.CustomLayerInterface, 'id' | 'type'> {
program: WebGLProgram | null = null;
aPos: number = 0;
buffer: WebGLBuffer | null = null;

onAdd(_map: maplibregl.Map, gl: WebGL2RenderingContext) {
//create GLSL source for vertex shader
const vertexSource = `#version 300 es
uniform mat4 u_matrix;
in vec2 a_pos;
void main() {
gl_Position = u_matrix * vec4(a_pos, 0.0, 1.0);
}`;

// create GLSL source for fragment shader
const fragmentSource = `#version 300 es
out highp vec4 fragColor;
void main() {
fragColor = vec4(1.0, 0.0, 0.0, 0.5);
}`;

// create a vertex shader
const vertexShader = gl.createShader(gl.VERTEX_SHADER)!;
gl.shaderSource(vertexShader, vertexSource);
gl.compileShader(vertexShader);

// create a fragment shader
const fragmentShader = gl.createShader(gl.FRAGMENT_SHADER)!;
gl.shaderSource(fragmentShader, fragmentSource);
gl.compileShader(fragmentShader);

// link the two shaders into a WebGL program
this.program = gl.createProgram()!;
gl.attachShader(this.program, vertexShader);
gl.attachShader(this.program, fragmentShader);
gl.linkProgram(this.program);
Comment on lines +27 to +40
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for shader compilation and program linking

To ensure the robustness of the application, it's important to check if the shaders compile successfully and if the program links correctly. Without these checks, shader compilation or linking failures may go unnoticed, making debugging difficult.

Apply this diff to add error handling:

 gl.compileShader(vertexShader);
+if (!gl.getShaderParameter(vertexShader, gl.COMPILE_STATUS)) {
+  console.error('Vertex shader compilation failed:', gl.getShaderInfoLog(vertexShader));
+}

 gl.compileShader(fragmentShader);
+if (!gl.getShaderParameter(fragmentShader, gl.COMPILE_STATUS)) {
+  console.error('Fragment shader compilation failed:', gl.getShaderInfoLog(fragmentShader));
+}

 gl.linkProgram(this.program);
+if (!gl.getProgramParameter(this.program, gl.LINK_STATUS)) {
+  console.error('Program linking failed:', gl.getProgramInfoLog(this.program));
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const vertexShader = gl.createShader(gl.VERTEX_SHADER)!;
gl.shaderSource(vertexShader, vertexSource);
gl.compileShader(vertexShader);
// create a fragment shader
const fragmentShader = gl.createShader(gl.FRAGMENT_SHADER)!;
gl.shaderSource(fragmentShader, fragmentSource);
gl.compileShader(fragmentShader);
// link the two shaders into a WebGL program
this.program = gl.createProgram()!;
gl.attachShader(this.program, vertexShader);
gl.attachShader(this.program, fragmentShader);
gl.linkProgram(this.program);
const vertexShader = gl.createShader(gl.VERTEX_SHADER)!;
gl.shaderSource(vertexShader, vertexSource);
gl.compileShader(vertexShader);
if (!gl.getShaderParameter(vertexShader, gl.COMPILE_STATUS)) {
console.error('Vertex shader compilation failed:', gl.getShaderInfoLog(vertexShader));
}
// create a fragment shader
const fragmentShader = gl.createShader(gl.FRAGMENT_SHADER)!;
gl.shaderSource(fragmentShader, fragmentSource);
gl.compileShader(fragmentShader);
if (!gl.getShaderParameter(fragmentShader, gl.COMPILE_STATUS)) {
console.error('Fragment shader compilation failed:', gl.getShaderInfoLog(fragmentShader));
}
// link the two shaders into a WebGL program
this.program = gl.createProgram()!;
gl.attachShader(this.program, vertexShader);
gl.attachShader(this.program, fragmentShader);
gl.linkProgram(this.program);
if (!gl.getProgramParameter(this.program, gl.LINK_STATUS)) {
console.error('Program linking failed:', gl.getProgramInfoLog(this.program));
}


this.aPos = gl.getAttribLocation(this.program, 'a_pos');

// define vertices of the triangle to be rendered in the custom style layer
const helsinki = maplibregl.MercatorCoordinate.fromLngLat({ lng: 25.004, lat: 60.239 });
const berlin = maplibregl.MercatorCoordinate.fromLngLat({ lng: 13.403, lat: 52.562 });
const kyiv = maplibregl.MercatorCoordinate.fromLngLat({ lng: 30.498, lat: 50.541 });

console.log(helsinki, berlin, kyiv);

// create and initialize a WebGLBuffer to store vertex and color data
this.buffer = gl.createBuffer();
gl.bindBuffer(gl.ARRAY_BUFFER, this.buffer);
gl.bufferData(
gl.ARRAY_BUFFER,
new Float32Array([helsinki.x, helsinki.y, berlin.x, berlin.y, kyiv.x, kyiv.y]),
gl.STATIC_DRAW
);
}

// method fired on each animation frame
render(gl: WebGL2RenderingContext | WebGLRenderingContext, options: maplibregl.CustomRenderMethodInput) {
gl.useProgram(this.program);
gl.uniformMatrix4fv(
gl.getUniformLocation(this.program!, 'u_matrix'),
false,
options.defaultProjectionData.mainMatrix
);
gl.bindBuffer(gl.ARRAY_BUFFER, this.buffer);
gl.enableVertexAttribArray(this.aPos);
gl.vertexAttribPointer(this.aPos, 2, gl.FLOAT, false, 0, 0);
gl.enable(gl.BLEND);
gl.blendFunc(gl.SRC_ALPHA, gl.ONE_MINUS_SRC_ALPHA);
gl.drawArrays(gl.TRIANGLE_STRIP, 0, 3);
}
Comment on lines +60 to +73
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure compatibility with WebGL1 contexts

The render method accepts both WebGL2RenderingContext and WebGLRenderingContext, but the shaders use WebGL2-specific features (e.g., #version 300 es). If the context is WebGL1, the shaders will fail to compile. Please ensure that the application handles cases where gl is a WebGL1 context or enforce that a WebGL2 context is used.

Consider updating the code to check if gl is a WebGL2RenderingContext and handle accordingly.

}

const customLayerImpl = new CustomLayerImpl();
</script>

<MapLibre
class="h-[60vh] min-h-[300px]"
style="https://basemaps.cartocdn.com/gl/voyager-gl-style/style.json"
zoom={3}
center={[20, 58]}
antialias
>
<CustomLayer implementation={customLayerImpl} />
</MapLibre>
15 changes: 15 additions & 0 deletions src/content/examples/custom-layer/content.svelte.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
title: Custom style layer
description: Use a custom style layer to render custom WebGL content.
---

<script lang="ts">
import Demo from "./CustomLayer.svelte";
import demoRaw from "./CustomLayer.svelte?raw";
import CodeBlock from "../../CodeBlock.svelte";
let { shiki } = $props();
</script>

<Demo />

<CodeBlock content={demoRaw} shiki={shiki} />
7 changes: 4 additions & 3 deletions src/content/examples/custom-protocol/content.svelte.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ description: How to add custom protocols.
---

<script lang="ts">
import CustomProtocol from "./CustomProtocol.svelte";
import Demo from "./CustomProtocol.svelte";
import demoRaw from "./CustomProtocol.svelte?raw";
import CodeBlock from "../../CodeBlock.svelte";
let { shiki } = $props();
</script>

<CustomProtocol />
<Demo />

<CodeBlock content={demoRaw} />
<CodeBlock content={demoRaw} {shiki} />
1 change: 1 addition & 0 deletions src/lib/maplibre/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export { default as HeatmapLayer } from './layers/HeatmapLayer.svelte';
export { default as RasterLayer } from './layers/RasterLayer.svelte';
export { default as HillshadeLayer } from './layers/HillshadeLayer.svelte';
export { default as BackgroundLayer } from './layers/BackgroundLayer.svelte';
export { default as CustomLayer } from './layers/CustomLayer.svelte';

// markers
export { default as Marker } from './markers/Marker.svelte';
Expand Down
84 changes: 84 additions & 0 deletions src/lib/maplibre/layers/CustomLayer.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
<script lang="ts">
// https://maplibre.org/maplibre-gl-js/docs/API/interfaces/CustomLayerInterface/

import { onDestroy, type Snippet } from 'svelte';
import maplibregl from 'maplibre-gl';
import { getMapContext } from '../contexts.svelte.js';
import { generateLayerID, resetLayerEventListener } from '../utils.js';
import type { MapLayerEventProps } from './common.js';

interface Props extends MapLayerEventProps {
id?: string;
beforeId?: string;
implementation: Omit<maplibregl.CustomLayerInterface, 'id' | 'type'>;
children?: Snippet;
}

let {
id: _id,
beforeId,
implementation,
children,

// Events
// https://maplibre.org/maplibre-gl-js/docs/API/type-aliases/MapLayerEventType/
onclick,
ondblclick,
onmousedown,
onmouseup,
onmousemove,
onmouseenter,
onmouseleave,
onmouseover,
onmouseout,
oncontextmenu,
ontouchstart,
ontouchend,
ontouchcancel
}: Props = $props();
Comment on lines +10 to +38
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix props declaration to align with Svelte's syntax

The current props declaration using $props() is not standard in Svelte and may cause syntax errors. In Svelte, you should use export let to declare component props.

Apply this diff to correct the props declaration:

-interface Props extends MapLayerEventProps {
-  id?: string;
-  beforeId?: string;
-  implementation: Omit<maplibregl.CustomLayerInterface, 'id' | 'type'>;
-  children?: Snippet;
-}
-
-let {
-  id: _id,
-  beforeId,
-  implementation,
-  children,
-
-  // Events
-  // https://maplibre.org/maplibre-gl-js/docs/API/type-aliases/MapLayerEventType/
-  onclick,
-  ondblclick,
-  onmousedown,
-  onmouseup,
-  onmousemove,
-  onmouseenter,
-  onmouseleave,
-  onmouseover,
-  onmouseout,
-  oncontextmenu,
-  ontouchstart,
-  ontouchend,
-  ontouchcancel
-}: Props = $props();
+export let id: string | undefined;
+const _id = id;
+export let beforeId: string | undefined;
+export let implementation: Omit<maplibregl.CustomLayerInterface, 'id' | 'type'>;
+export let children: Snippet | undefined;
+
+// Events
+// https://maplibre.org/maplibre-gl-js/docs/API/type-aliases/MapLayerEventType/
+export let onclick;
+export let ondblclick;
+export let onmousedown;
+export let onmouseup;
+export let onmousemove;
+export let onmouseenter;
+export let onmouseleave;
+export let onmouseover;
+export let onmouseout;
+export let oncontextmenu;
+export let ontouchstart;
+export let ontouchend;
+export let ontouchcancel;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
interface Props extends MapLayerEventProps {
id?: string;
beforeId?: string;
implementation: Omit<maplibregl.CustomLayerInterface, 'id' | 'type'>;
children?: Snippet;
}
let {
id: _id,
beforeId,
implementation,
children,
// Events
// https://maplibre.org/maplibre-gl-js/docs/API/type-aliases/MapLayerEventType/
onclick,
ondblclick,
onmousedown,
onmouseup,
onmousemove,
onmouseenter,
onmouseleave,
onmouseover,
onmouseout,
oncontextmenu,
ontouchstart,
ontouchend,
ontouchcancel
}: Props = $props();
export let id: string | undefined;
const _id = id;
export let beforeId: string | undefined;
export let implementation: Omit<maplibregl.CustomLayerInterface, 'id' | 'type'>;
export let children: Snippet | undefined;
// Events
// https://maplibre.org/maplibre-gl-js/docs/API/type-aliases/MapLayerEventType/
export let onclick;
export let ondblclick;
export let onmousedown;
export let onmouseup;
export let onmousemove;
export let onmouseenter;
export let onmouseleave;
export let onmouseover;
export let onmouseout;
export let oncontextmenu;
export let ontouchstart;
export let ontouchend;
export let ontouchcancel;


const mapCtx = getMapContext();
if (!mapCtx.map) throw new Error('Map instance is not initialized.');

const id = _id ?? generateLayerID();

(implementation as maplibregl.CustomLayerInterface).id ??= id;
(implementation as maplibregl.CustomLayerInterface).type = 'custom';
Comment on lines +45 to +46
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid mutating the implementation object directly

Mutating the implementation object by adding or modifying properties like id and type may lead to unintended side effects, especially if the object is used elsewhere. Consider creating a new object to prevent potential issues.

Apply this diff to create a new implementation object:

-(implementation as maplibregl.CustomLayerInterface).id ??= id;
-(implementation as maplibregl.CustomLayerInterface).type = 'custom';
+
+const layerImplementation: maplibregl.CustomLayerInterface = {
+  ...implementation,
+  id: implementation.id ?? id,
+  type: 'custom',
+};

And update the addLayer call:

-mapCtx.addLayer(implementation as maplibregl.CustomLayerInterface, beforeId);
+mapCtx.addLayer(layerImplementation, beforeId);

Committable suggestion skipped: line range outside the PR's diff.


let firstRun = true;
mapCtx.waitForStyleLoaded(() => {
mapCtx.addLayer(implementation as maplibregl.CustomLayerInterface, beforeId);
});

$effect(() => resetLayerEventListener(mapCtx.map, 'click', id, onclick));
$effect(() => resetLayerEventListener(mapCtx.map, 'dblclick', id, ondblclick));
$effect(() => resetLayerEventListener(mapCtx.map, 'mousedown', id, onmousedown));
$effect(() => resetLayerEventListener(mapCtx.map, 'mouseup', id, onmouseup));
$effect(() => resetLayerEventListener(mapCtx.map, 'mousemove', id, onmousemove));
$effect(() => resetLayerEventListener(mapCtx.map, 'mouseenter', id, onmouseenter));
$effect(() => resetLayerEventListener(mapCtx.map, 'mouseleave', id, onmouseleave));
$effect(() => resetLayerEventListener(mapCtx.map, 'mouseover', id, onmouseover));
$effect(() => resetLayerEventListener(mapCtx.map, 'mouseout', id, onmouseout));
$effect(() => resetLayerEventListener(mapCtx.map, 'contextmenu', id, oncontextmenu));
$effect(() => resetLayerEventListener(mapCtx.map, 'touchstart', id, ontouchstart));
$effect(() => resetLayerEventListener(mapCtx.map, 'touchend', id, ontouchend));
$effect(() => resetLayerEventListener(mapCtx.map, 'touchcancel', id, ontouchcancel));

$effect(() => {
if (beforeId && !firstRun) {
mapCtx.waitForStyleLoaded((map) => {
map.moveLayer(id, beforeId);
});
}
});

$effect(() => {
firstRun = false;
});

onDestroy(() => {
mapCtx.removeLayer(id);
});
</script>

{@render children?.()}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the rendering of child content

The syntax {@render children?.()} is not valid in Svelte. To render child content or slots, you should use <slot /> or directly invoke the function if children is a render function.

Apply this diff to fix the rendering:

-{@render children?.()}
+{#if children}
+  {children()}
+{/if}

Alternatively, if you intend to use slots, consider replacing children with <slot />:

-{@render children?.()}
+<slot />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{@render children?.()}
{#if children}
{children()}
{/if}
Suggested change
{@render children?.()}
<slot />

4 changes: 2 additions & 2 deletions src/routes/components/+layout.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
let { children } = $props();
</script>

<div class="mx-auto grid grid-cols-[220px_minmax(0,1fr)] gap-10 px-8 2xl:container">
<div class="mx-auto grid grid-cols-[200px_minmax(0,1fr)] gap-10 px-8 2xl:container">
<aside class="sticky bottom-0 top-16 h-[calc(100vh-4rem)] overflow-scroll py-8">
<Index />
</aside>
<main class="grid-cols-[1fr_220px]">
<main class="grid-cols-[1fr_200px]">
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

The grid-cols class on the main element is missing the grid class

The main element has grid-cols-[1fr_200px] but lacks the required grid class to activate the grid layout. This is inconsistent with both:

  • The parent element which correctly uses grid grid-cols-[200px_minmax(0,1fr)]
  • The child page component which correctly uses grid grid-cols-[1fr_200px]

Either:

  1. Add the grid class if you want to maintain the grid layout: class="grid grid-cols-[1fr_200px]"
  2. Remove the grid-cols-[1fr_200px] class if no grid layout is needed
🔗 Analysis chain

Verify the grid configuration in the main element

The grid columns defined on the main element (grid-cols-[1fr_200px]) appear to be unused as there are no visible grid items. Additionally, this configuration differs from the parent grid's structure. Consider:

  1. Removing the grid columns if they're not needed
  2. Aligning with the parent grid's configuration if a grid is required
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any grid items or content that might justify the grid columns in main
rg -l 'col-span-|col-start-' src/routes/components/

Length of output: 53


Script:

#!/bin/bash
# Let's check the content of the layout file and its surrounding context
cat src/routes/components/+layout.svelte

# Also search for any grid-related classes in the components directory
rg "grid-" src/routes/components/ -A 2 -B 2

Length of output: 1466

{@render children()}
</main>
</div>
2 changes: 1 addition & 1 deletion src/routes/components/[slug]/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const { data } = $props();
</script>

<div class="grid grid-cols-[1fr_220px] gap-x-10">
<div class="grid grid-cols-[1fr_200px] gap-x-10">
<div class="min-h-[calc(100vh-4rem)] w-full min-w-0 py-8">
<h1 class="mb-2 text-3xl font-bold">{data.meta.title}</h1>

Expand Down
4 changes: 2 additions & 2 deletions src/routes/examples/+layout.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
let { children } = $props();
</script>

<div class="mx-auto grid grid-cols-[220px_minmax(0,1fr)] gap-10 px-8 2xl:container">
<div class="mx-auto grid grid-cols-[200px_minmax(0,1fr)] gap-10 px-8 2xl:container">
<aside class="sticky bottom-0 top-16 h-[calc(100vh-4rem)] overflow-scroll py-8">
<Index />
</aside>
<main class="grid-cols-[1fr_220px]">
<main class="grid-cols-[1fr_200px]">
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unnecessary grid-cols from main element

The grid-cols-[1fr_200px] class on the <main> element appears unnecessary as it's not displaying a grid layout and its children are rendered directly. This could be confusing for maintenance.

- <main class="grid-cols-[1fr_200px]">
+ <main>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<main class="grid-cols-[1fr_200px]">
<main>

{@render children()}
</main>
</div>
2 changes: 1 addition & 1 deletion src/routes/examples/[slug]/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const { data } = $props();
</script>

<div class="grid grid-cols-[1fr_220px] gap-x-10">
<div class="grid grid-cols-[1fr_200px] gap-x-10">
<div class="min-h-[calc(100vh-4rem)] w-full min-w-0 py-8">
<h1 class="mb-2 text-3xl font-bold">{data.meta.title}</h1>

Expand Down