Skip to content

Add base vertex uniform count.#2886

Merged
cptbtptpbcptdtptp merged 1 commit intogalacean:dev/2.0from
cptbtptpbcptdtptp:fix/baseVertexUniformCount
Jan 27, 2026
Merged

Add base vertex uniform count.#2886
cptbtptpbcptdtptp merged 1 commit intogalacean:dev/2.0from
cptbtptpbcptdtptp:fix/baseVertexUniformCount

Conversation

@cptbtptpbcptdtptp
Copy link
Collaborator

@cptbtptpbcptdtptp cptbtptpbcptdtptp commented Jan 27, 2026

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

Other information:

Summary by CodeRabbit

  • Refactor
    • Enhanced the resource allocation strategy for skinned mesh rendering to improve overall efficiency when handling complex mesh configurations with multiple joints and blend shapes. This optimization provides better support for devices with varying resource constraints and enables improved handling of edge cases in mesh rendering scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@cptbtptpbcptdtptp cptbtptpbcptdtptp added rendering Rendering related functions ignore for release ignore for release labels Jan 27, 2026
@cptbtptpbcptdtptp cptbtptpbcptdtptp self-assigned this Jan 27, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 27, 2026

Walkthrough

A new static member _baseVertexUniformVectorCount is introduced to SkinnedMeshRenderer with a value of 48, replacing a hard-coded constant 44 in the calculation of remainUniformJointCount. This refactoring centralizes the uniform vector count logic for improved maintainability.

Changes

Cohort / File(s) Summary
Static Member Extraction
packages/core/src/mesh/SkinnedMeshRenderer.ts
Introduced static member _baseVertexUniformVectorCount (value: 48) and updated remainUniformJointCount calculation to use this constant instead of hard-coded value 44. Changes affect uniform vector availability determination for joint texture versus macro selection.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A number once hidden, now stands tall and true,
Forty-eight vectors, extracted anew,
No more magic numbers to confuse and perplex,
Clean code hops forward, what's next, what's next? ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add base vertex uniform count' accurately describes the main change: introducing a new _baseVertexUniformVectorCount static member to replace the hard-coded constant 44 in uniform vector calculations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@augmentcode
Copy link

augmentcode bot commented Jan 27, 2026

🤖 Augment PR Summary

Summary: Extracts the base vertex-uniform-vector budget used for skinning into a shared constant and applies it in the joint-count calculation.
Changes: Adds SkinnedMeshRenderer._baseVertexUniformVectorCount (48) and uses it when computing how many joint matrices can fit in vertex uniforms to avoid unnecessary shader recompiles.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

// directly use max joint count to avoid shader recompile
// @TODO: different shader type should use different count, not always 44
const remainUniformJointCount = Math.ceil((this._maxVertexUniformVectors - (44 + bsUniformOccupiesCount)) / 4);
const remainUniformJointCount = Math.ceil(
Copy link

Choose a reason for hiding this comment

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

Using Math.ceil here can overestimate remainUniformJointCount when the available uniform-vector budget isn’t divisible by 4, which may make RENDERER_JOINTS_NUM exceed MAX_VERTEX_UNIFORM_VECTORS and cause shader compile failures on some devices. Consider rounding down instead of up so the computed joint limit always stays within the reported uniform budget.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

@cptbtptpbcptdtptp cptbtptpbcptdtptp merged commit 4bf1668 into galacean:dev/2.0 Jan 27, 2026
10 checks passed
@codecov
Copy link

codecov bot commented Jan 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.95%. Comparing base (78b06f9) to head (aa14a5f).
⚠️ Report is 1 commits behind head on dev/2.0.

Additional details and impacted files
@@             Coverage Diff             @@
##           dev/2.0    #2886      +/-   ##
===========================================
- Coverage    83.22%   82.95%   -0.28%     
===========================================
  Files          796      796              
  Lines        90434    90441       +7     
  Branches      9496     9476      -20     
===========================================
- Hits         75266    75027     -239     
- Misses       15086    15332     +246     
  Partials        82       82              
Flag Coverage Δ
unittests 82.95% <100.00%> (-0.28%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ignore for release ignore for release rendering Rendering related functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants