Skip to content

Conversation

@hun756
Copy link
Contributor

@hun756 hun756 commented Oct 24, 2025

No description provided.

@hun756 hun756 requested a review from Copilot October 24, 2025 15:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a comprehensive WebGL2 rendering system with support for textures, shaders, materials, meshes, and batch rendering. The implementation provides a complete abstraction layer over WebGL2 with modern features including PBR materials, shader variants, and optimized batch rendering.

Key Changes

  • Complete WebGL2 texture system with format info, validation, and utilities
  • Advanced shader compilation and management with variant support and caching
  • Material system supporting Standard and PBR workflows with property management
  • Mesh/geometry system with vertex/index buffers and VAO management
  • Batch rendering system for optimized draw call reduction

Reviewed Changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
packages/core/src/renderer/webgl2/texture/utils.ts Texture utility classes for format handling, WebGL constants mapping, and validation
packages/core/src/renderer/webgl2/texture/texture.ts Core WebGL texture implementation with mipmap generation and data upload
packages/core/src/renderer/webgl2/texture/sampler.ts Texture sampler implementation with filtering and wrapping configuration
packages/core/src/renderer/webgl2/texture/manager.ts Texture manager for creation, caching, and loading from files
packages/core/src/renderer/webgl2/shader/utils.ts Shader utilities for type sizes, validation, and buffer layout
packages/core/src/renderer/webgl2/shader/compiler.ts Shader compilation with variant support and validation
packages/core/src/renderer/webgl2/shader/material.ts Material instance with property and keyword management
packages/core/src/renderer/webgl2/shader/manager.ts Shader manager with caching and variant handling
packages/core/src/renderer/webgl2/mesh/utils.ts Mesh utilities for layout validation and bounds calculation
packages/core/src/renderer/webgl2/material/standard-material.ts Unity-style standard PBR material implementation
packages/core/src/renderer/webgl2/material/pbr-material.ts glTF 2.0 compatible PBR material implementation
packages/core/src/renderer/webgl2/batch/batch-renderer.ts Batch renderer for optimized instanced rendering

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

}

public static generateTextureId(): string {
return `tex_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The substr method is deprecated. Use substring or slice instead.

Suggested change
return `tex_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
return `tex_${Date.now()}_${Math.random().toString(36).slice(2, 11)}`;

Copilot uses AI. Check for mistakes.
}

if (TextureFormatInfo.isInteger(format)) {

Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Empty if block should either be removed or contain a TODO comment explaining why it's intentionally empty.

Suggested change
// TODO: Handle integer format compatibility if needed in future.

Copilot uses AI. Check for mistakes.
Comment on lines +421 to +424
const r = parseInt(hex.substr(0, 2), 16) / 255;
const g = parseInt(hex.substr(2, 2), 16) / 255;
const b = parseInt(hex.substr(4, 2), 16) / 255;
const a = hex.length === 8 ? parseInt(hex.substr(6, 2), 16) / 255 : 1;
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The substr method is deprecated. Use substring or slice instead.

Suggested change
const r = parseInt(hex.substr(0, 2), 16) / 255;
const g = parseInt(hex.substr(2, 2), 16) / 255;
const b = parseInt(hex.substr(4, 2), 16) / 255;
const a = hex.length === 8 ? parseInt(hex.substr(6, 2), 16) / 255 : 1;
const r = parseInt(hex.slice(0, 2), 16) / 255;
const g = parseInt(hex.slice(2, 4), 16) / 255;
const b = parseInt(hex.slice(4, 6), 16) / 255;
const a = hex.length === 8 ? parseInt(hex.slice(6, 8), 16) / 255 : 1;

Copilot uses AI. Check for mistakes.
}

private _generateSamplerId(): string {
return `sampler_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The substr method is deprecated. Use substring or slice instead.

Suggested change
return `sampler_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
return `sampler_${Date.now()}_${Math.random().toString(36).substring(2, 11)}`;

Copilot uses AI. Check for mistakes.

if (this.options.borderColor) {
const color = this.options.borderColor;

Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Empty if block should either be removed or contain a TODO comment explaining why border color configuration is not yet implemented.

Suggested change
// TODO: Border color configuration is not yet implemented.
// WebGL2 does not support sampler border color directly.

Copilot uses AI. Check for mistakes.
}

if (this.options.lodBias !== undefined) {

Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Empty if block should either be removed or contain a TODO comment explaining why LOD bias configuration is not yet implemented.

Suggested change
// TODO: LOD bias configuration is not yet implemented.

Copilot uses AI. Check for mistakes.
options?: { defines?: Record<string, any>; version?: string; precision?: string; }
): Promise<string> {
return new Promise((resolve, reject) => {
const id = `${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The substr method is deprecated. Use substring or slice instead.

Suggested change
const id = `${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
const id = `${Date.now()}_${Math.random().toString(36).slice(2, 11)}`;

Copilot uses AI. Check for mistakes.
}

public static generateMeshId(): string {
return `mesh_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The substr method is deprecated. Use substring or slice instead.

Suggested change
return `mesh_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
return `mesh_${Date.now()}_${Math.random().toString(36).slice(2, 11)}`;

Copilot uses AI. Check for mistakes.
this.bufferFactory = createBufferFactory(gl);
this._usage = config.usage || BufferUsage.STATIC_DRAW;
this._indexType = config.indexType || IndexType.UNSIGNED_SHORT;
this._id = `index_buffer_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The substr method is deprecated. Use substring or slice instead.

Suggested change
this._id = `index_buffer_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
this._id = `index_buffer_${Date.now()}_${Math.random().toString(36).slice(2, 11)}`;

Copilot uses AI. Check for mistakes.
hun756 and others added 2 commits October 24, 2025 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant