jscad / OpenJSCAD.org

JSCAD is an open source set of modular, browser and command line tools for creating parametric 2D and 3D designs with JavaScript code. It provides a quick, precise and reproducible method for generating 3D models, and is especially useful for 3D printing applications.
https://openjscad.xyz/
MIT License
2.65k stars 513 forks source link

Incorrect lighting on translated object #985

Open platypii opened 2 years ago

platypii commented 2 years ago

A lighting bug was introduced recently. Models that used to render correctly started showing "double lighting" across parallel surfaces.

Expected Behavior

Consistent global lighting as shown here in @jscad/web@2.4.3 and before:

lighting-good

Actual Behavior

A double reflection of the light source on the translated cylinder on @jscad/web@2.4.4 and after:

lighting-bad

Steps to Reproduce the Problem

const jscad = require("@jscad/modeling")
const { cylinder } = jscad.primitives
const { rotateY, translate } = jscad.transforms

function main() {
  const radius = 20
  const height = 80
  const segments = 80
  return [
    rotateY(Math.PI / 2, cylinder({radius, height, segments})),
    rotateY(Math.PI / 2, cylinder({radius, height, segments, center: [0, 0, 82]})), // good lighting
    translate([-82, 0, 0], rotateY(Math.PI / 2, cylinder({radius, height, segments}))), // bad lighting
  ]
}

module.exports = { main }

The problem only appears on the translated cylinder, not the re-centered cylinder. Also worth noting that these are all independently constructed objects, so this does not appear to be a caching, or copying bug.

Specifications

platypii commented 2 years ago

I used git bisect to track the regression specifically to commit b29993a8 from PR #928

It appears there was previous discussion of lighting bugs between @z3dev and @hrgdavor in #928 but maybe was not fully resolved.

hrgdavor commented 2 years ago

Yes, I am new to shaders and WEBGL, just started to learn it to be able to bring instancing support to regl renderer.

I tried to change old shader to work properly with geometries that have transformation for shader to transform vertices while rendering. The lighting on the old shader was a bit strange so I had problems replicating it.

It was a lot of guess work to convert the existing shader to use transformations and render decently.

I am currently working on a new prototype #944 that will be able to swap between

I may take while until I have time to try to tackle this. But a PR is always welcome :)

platypii commented 2 years ago

Should the transforms be applied before it gets to the shader? Or should the transforms be passed in and applied inside the shader? Is vColorShaders.js the relevant code?

hrgdavor commented 2 years ago

Should the transforms be applied before it gets to the shader? Or should the transforms be passed in and applied inside the shader? Is vColorShaders.js the relevant code?

Yes, I think vColorShaders.js is the code I tweaked.

for instancing to work shader must be able to apply transforms. just look at cpu time if you calculate transform for vertices of 100 spheres, and also amount data in buffers to send to GPU... instead send one sphere and 100 transforms.

try to do this in regl renderere in jscad with more than 5x5x5

http://3d.hrg.hr/jscad/three/threejscad2.html?uri=model.logos.js