Chore ci snap - #2761
Conversation
🦋 Changeset detectedLatest commit: 2e4daba The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello @lzxue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the L7 mapping library by introducing new examples for advanced functionalities like map image export and PMTiles integration. It also includes several targeted bug fixes and code quality improvements across different packages, contributing to a more robust and user-friendly development experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several new examples, including map image export and PMTiles support, along with various fixes and improvements across the codebase. My review focuses on improving the correctness and maintainability of the new examples, such as fixing a bug in the image export logic and making asynchronous operations more robust. I've also pointed out areas where debug code like console.log should be removed and where error handling and code assumptions can be strengthened for better stability.
| // @ts-ignore | ||
| const format = formatSelect.value as 'png' | 'jpg'; | ||
| //@ts-ignore | ||
| console.log('format', scene.mapService); | ||
| //@ts-ignore | ||
| const dataURL = await scene.mapService.exportMap('png'); | ||
| // 使用场景的导出方法 | ||
| // const dataURL = await scene.exportMap(format) |
There was a problem hiding this comment.
The image export functionality has a few issues:
- The image format is hardcoded to
'png'inscene.mapService.exportMap('png'), ignoring the user's selection from the dropdown. Theformatvariable should be used instead. - There are several
@ts-ignorecomments and aconsole.logwhich should be removed from production/example code. - The commented-out line
// const dataURL = await scene.exportMap(format)suggests the correct method might be on thesceneobject itself. Please ensure you are using the correct API.
const format = formatSelect.value as 'png' | 'jpg';
const dataURL = await scene.mapService.exportMap(format);| '@antv/l7': patch | ||
| --- | ||
|
|
||
| 修复一些bug |
There was a problem hiding this comment.
The description "修复一些bug" (fix some bugs) is too generic. For better maintainability and clearer release notes, please provide a more specific summary of the bugs that were fixed in this changeset.
| await new Promise((resolve) => { | ||
| const checkRender = () => { | ||
| if (scene.getLayers().every((layer) => layer.isVisible())) { | ||
| resolve(true); | ||
| } else { | ||
| setTimeout(checkRender, 100); | ||
| } | ||
| }; | ||
| checkRender(); | ||
| }); |
There was a problem hiding this comment.
Using setTimeout in a loop to check if a layer is rendered is inefficient and can be unreliable. A better approach is to use the inited event on the layer, which signals that it's ready.
await new Promise<void>((resolve) => {
pointLayer.once('inited', () => {
resolve();
});
});| //@ts-ignore | ||
| Scene.addProtocol('pmtiles', protocol.tile); |
There was a problem hiding this comment.
The @ts-ignore comment should be avoided. If Scene.addProtocol is a valid method but lacks type definitions, consider casting Scene to any like (Scene as any).addProtocol(...) for clarity. Even better would be to update the type definitions for Scene to include addProtocol if it's part of the public API.
| //@ts-ignore | |
| Scene.addProtocol('pmtiles', protocol.tile); | |
| (Scene as any).addProtocol('pmtiles', protocol.tile); |
| //@ts-ignore | ||
| Scene.addProtocol('pmtiles', protocol.tile); |
There was a problem hiding this comment.
The @ts-ignore comment should be avoided. If Scene.addProtocol is a valid method but lacks type definitions, consider casting Scene to any like (Scene as any).addProtocol(...) for clarity. Even better would be to update the type definitions for Scene to include addProtocol if it's part of the public API.
| //@ts-ignore | |
| Scene.addProtocol('pmtiles', protocol.tile); | |
| (Scene as any).addProtocol('pmtiles', protocol.tile); |
| // 对于 ArrayBuffer 或其他数据类型，需要提供宽度和高度 | ||
| const width = Math.sqrt(data.length / 4); // 假设是RGBA格式，每个像素4个值 | ||
| const height = width; | ||
| gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, width, height, 0, gl.RGBA, gl.UNSIGNED_BYTE, data); |
There was a problem hiding this comment.
The logic to infer texture dimensions assumes the texture is always square (width = height = Math.sqrt(...)). This is fragile and will fail if the texture data is not from a square texture, as Math.sqrt could produce a non-integer. Consider adding a check to ensure the calculated width is an integer, or refactoring to have width and height passed in as parameters, similar to the createTexture function.
// 对于 ArrayBuffer 或其他数据类型，需要提供宽度和高度
const width = Math.sqrt(data.length / 4); // 假设是RGBA格式，每个像素4个值
if (!Number.isInteger(width)) {
console.warn('Texture data is not a square, which may cause rendering issues.');
}
const height = width;
gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, width, height, 0, gl.RGBA, gl.UNSIGNED_BYTE, data);| transform._calcMatrices(); | ||
|
|
||
| expect(transform.customLayerMatrix()[0].toString().length).toBeGreaterThan(10); | ||
| console.log('----->1', transform.customLayerMatrix().length); |
There was a problem hiding this comment.
This console.log statement appears to be for debugging and should be removed from the test case.
| console.log('renderCanvas', renderCanvas); | ||
| if (!renderCanvas) { | ||
| throw new Error('地图尚未初始化完成，无法导出图片'); | ||
| } |
There was a problem hiding this comment.
There are a couple of issues here:
- A
console.logstatement has been added, which should be removed. - The error message is in Chinese. For consistency with the rest of the codebase, please use English for error messages.
if (!renderCanvas) {
throw new Error('Map has not been initialized yet, unable to export image.');
}
1 participant
[English Template / 英文模板]
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
📝 更新日志
☑️ 请求合并前的自查清单