playcanvas / playcanvas/engine

Toksvig specular doesn't work

Open
#4,260 9 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area: graphics
Dominant language
JavaScript
Stars
16.8k
Forks
2k
Avg merge
4h 32m
Merged PRs (30d)
222

Description

Toksvig specular antialiasing has been broken in the engine for the last two years.

This was unintentionally broken when detail normal maps were added to the engine (see https://github.com/playcanvas/engine/pull/1968/files#diff-dc5bc7097c897bc531ef1e99641a4b3dbd3f70b96fd0c906e264c6f540771414R5).

Toksvig factor is based on the length of the dNormalMap vector as read from the texture/mipmap. The normalise call on this line results in length always being 1.0, effectively disabling toksvig.

We must decide how to proceed: either we re-enable toksvig support in the engine or we remote it.

Keep
Advantages
  • less specular aliasing, simple implementation
  • works with most gltf assets
  • essentially "free" since we've always executed the shader math
    Disadvantage
    • re-introducing toksvig has the potential to change lighting/specular in existing projects
    • looks significantly different to vanilla/non-toksvig specular
    • only works with blinn-phong anyway and we'll likely move to GGX in future
    • toksvig only works in some cases:
      • doesn't work with XY/basis normals maps (which is required for high-quality compressed normals)
      • only impacts punctual lights, doesn't address geometric-based aliasing
      • requires valid normals maps (containing normalised normals)
    Remove
    Advantages
    • no impact to existing projects
    • shaders will be smaller, faster
    • simplify chunks
    Disadvantages
    • continue to live with more aliasing

    Contributor guide

    Open the contributing guide

    First steps

    1. Read the whole issue, then the project's contributing guide.
    2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
    3. Fork the repository and make your change on a branch.
    4. Open a pull request that references the issue number.

    Research direction

    Start with the linked pull request diff and the shader code around the dNormalMap normalise call. Determine whether Toksvig support should be restored or removed, then verify that the chosen direction addresses the stated specular aliasing and compatibility concerns.

    Written by the indexing model from the issue text.

    Assessment

    Tech stack
    javascript
    Domain
    computer-graphics
    Issue type
    Bug
    Difficulty
    5/5
    Estimated time
    Over a week
    Activity status
    Stale
    Clarity
    Needs clarification
    Newbie friendliness
    25/100

    Get new issues in your inbox

    A short digest of beginner-friendly GitHub issues.