molstar / molstar

A comprehensive macromolecular library
https://molstar.org
MIT License
596 stars 139 forks source link

Bumpiness not working on ball and stick representation #1084

Open giagitom opened 1 month ago

giagitom commented 1 month ago

It does not work using both meshes or impostors

arose commented 1 month ago

That is intended, the default bumpFrequency is set to zero for some representations where is does not look good. See src\mol-repr\structure\representation\ball-and-stick.ts. If you change the bumpFrequency you can still get bumpiness.

giagitom commented 1 month ago

Hmm.. It seems that also varying bump frequency does not produce any effect on the representation. Maybe it is disabled somewhere else?

image

giagitom commented 1 month ago

Oh.. sorry It was my fault. I didn't increase bumpiness on material. It is working fine.

Maybe just reconsider enable it by default since I hardly see any graphic artifacts or issues.

image

The only issue I can see is this in the contact between cylinders.. but it is super small image

arose commented 1 month ago

I find the artifact quite strong without specular highlights

image

arose commented 1 month ago

If you can fix the artifact, I am happy to reconsider

giagitom commented 1 month ago

@arose I have found an other related issue with bumpiness on impostor spheres and cylinders that is most visible when xRayShaded is on. Here is a test state to reproduce the issue test.molj.txt

image

The issue seems to be due to this piece of code inside perturbNormal https://github.com/molstar/molstar/blob/9604b89ee0716442b0bc8b862cfe712468dc6c95/src/mol-gl/shader/chunks/common-frag-params.glsl.ts#L114

Handling edge cases in which det == 0 like this if (det == 0.0) return vec3(normal); seems to fix the problem and also reduces the effect of the before mentioned artifact (that is still present though).

I suspect there is an other similar edge case breaking the perturbed normal.

arose commented 3 weeks ago

thanks, applied the fix in fda3481d1504213dbabf70ec264580df2dfb22ff