feat: 新增 PointLayer shape: image 新增 allowOverlap 配置,用于控制图片之间自动避让隐藏 - #2828
Conversation
|
Summary of ChangesHello, 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! 此拉取请求的核心目标是增强 PointLayer 在显示图片图标时的用户体验,通过引入 Highlights
Changelog
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
|
There was a problem hiding this comment.
Code Review
本次 PR 新增了 PointLayer 中 shape: image 类型的 allowOverlap 配置,用于控制图片标注的碰撞避让,这是一个很有用的功能。
代码实现方面,核心逻辑主要在 packages/layers/src/point/models/image.ts 中,通过 CollisionIndex 在屏幕空间进行 AABB 碰撞检测,并在 PointImageTriangulation 中过滤掉被遮挡的图标,整体思路清晰。
我发现了一些可以改进的地方:
- 在
image.ts中,buildModels和reBuildModel方法存在代码重复,可以提取一个私有方法来优化。 - 同样在
image.ts中,对没有id的 feature 赋予默认值0可能会导致多个 feature 共享同一id,从而在碰撞检测中出现问题。 - 在
triangulation.ts中,使用了@ts-ignore来处理this的类型,有更优雅的 TypeScript 写法。
具体的修改建议请见代码评论。除此之外,文档和示例的更新也很完整,做得很好。
| const data = this.layer.getEncodedData(); | ||
|
|
||
| data.forEach((feature: IEncodeFeature) => { | ||
| const { id = 0, size = 5 } = feature; |
There was a problem hiding this comment.
此处为 id 设置默认值 0 可能会导致问题。如果多个 feature 的 id 都是 undefined,它们都会被赋予 id = 0。这会导致在 imageFilterMap 和 collisionIndex 中互相覆盖,使得只有一个 id 为 0 的 feature 能够被正确处理,而其他没有 id 的 feature 会被错误地隐藏或显示。
建议修改此行,并在下一行添加对 id 是否为 undefined 的检查,如果是则跳过当前 feature:
if (id === undefined) {
// console.warn('Feature without id is skipped in collision detection.');
return;
}| const { id = 0, size = 5 } = feature; | |
| const { id, size = 5 } = feature; |
| // @ts-ignore | ||
| const that = this as { imageFilterMap?: { [id: number]: boolean } }; |
There was a problem hiding this comment.
| private async reBuildModel() { | ||
| const { allowOverlap = true } = this.layer.getLayerConfig() as IPointLayerStyleOptions; | ||
| if (!allowOverlap) { | ||
| this.filterImages(); | ||
| } else { | ||
| this.imageFilterMap = null; // 允许压盖:清空过滤表,全量渲染 | ||
| } | ||
| const model = await this.layer.buildLayerModel({ | ||
| moduleName: 'pointImage', | ||
| vertexShader: pointImageVert, | ||
| fragmentShader: pointImageFrag, | ||
| triangulation: PointImageTriangulation.bind(this), | ||
| defines: this.getDefines(), | ||
| inject: this.getInject(), | ||
| depth: { enable: false }, | ||
| primitive: gl.POINTS, | ||
| }); | ||
| this.layer.models = [model]; | ||
| } |
There was a problem hiding this comment.
此方法 reBuildModel 与 buildModels 方法存在大量重复代码。为了提高代码的可维护性,建议提取一个私有的辅助方法来封装公共的模型创建逻辑,以减少重复并使代码更清晰。
例如,可以创建一个 _buildModel 方法:
private async _buildModel(): Promise<IModel> {
const { allowOverlap = true } = this.layer.getLayerConfig() as IPointLayerStyleOptions;
if (!allowOverlap) {
this.filterImages();
} else {
this.imageFilterMap = null;
}
return await this.layer.buildLayerModel({
moduleName: 'pointImage',
vertexShader: pointImageVert,
fragmentShader: pointImageFrag,
triangulation: PointImageTriangulation.bind(this),
defines: this.getDefines(),
inject: this.getInject(),
depth: { enable: false },
primitive: gl.POINTS,
});
}然后 buildModels 和 reBuildModel 可以简化为:
public async buildModels(): Promise<IModel[]> {
this.initUniformsBuffer();
const model = await this._buildModel();
return [model];
}
private async reBuildModel() {
const model = await this._buildModel();
this.layer.models = [model];
}
[English Template / 英文模板]
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
📝 更新日志
☑️ 请求合并前的自查清单