JDK-8217472: 3D-API - Attenuation-coefficients for PointLight

[ProggerFox]

Good evening folks,

while playing on a 3d scene, i noticed that the PointLight (very specific, maybe other use-cases for it might come up too) lacked a way to set the radius of how far the light is emitted from the light-source.

Regarding this i was hoping there would be a operation like the "setInfluencingBounds"-method (hence the title) as in the predecessor Java3D available, since the PointLights-emission procedure seems to also rely on its BoundTransforms.
Sadly there is no way of modifying this bit and therefore the PointLight seems rather limited because of it, because not every lamp or similiar light-entity emits light in the same manner.

Maybe i overlooked certain steps and someone might take some time to elaborate on how to take on this issue, but else i would assume it to be a missing feature.

Thanks in advance!

[nlisker]

What you are looking for are attenuation factors. These need to be added at the native graphics pipelines level, which is a good amount of work. I can help with the D3D pipeline, but someone else has to do the others.

As a workaround, you can find all the nodes a certain distance from your source and add them to the light's scope. It's not perfect, but it's something.

Lighting and materials in general can use a lot of work, like adding directional lights and spotlights, an emissive component etc.

[ProggerFox]

Thanks for your reply,

it would be very appreciated if you could help out with the D3D pipeline.
Luckily i also dealed with OpenGL a (longer) while back, so i would gladly help out with the ES2 pipeline.
I never heard about the SW/(Pisces?) pipeline though and cant find references quickly at the moment, so hopefully someone will be able to deal with this pipeline as well.

The idea with adding the nodes to the scope came up to me as well, though the core problem of the light-emission still remains, with only the nodes in the scope being affected then of course.
As closest to the result i could get was via a self-illuminated object (by using a Sphere with a illumination-texture in a material), though it was more of a hack than a full-fledged solution.
Working on a native solution instead seems to be a better time spent in the end though.

Well, i would gladly try my luck to work on these missing components then, since it is also in my interest to enchance the capabilities of my favourite framework :)

[nlisker]

SW is the software pipeline and it doesn't support 3D. Note that Apple are phasing out OpenGL support in favor of Metal.

We'll need:

• A concrete proposal of what we want to get done. I suggest we start from something simple like attenuation factors for the current point light implementation and see how that goes (see caveat below). When we have that I'll open a JBS issue.
• To gather all the interested people who can help; a mail on the discussion mailing list ([email protected]) is a good place to start as this has been brought up before in some form or another.
• A guarantee that this isn't a drive-by feature. That means that the contributors will provide support for this feature in the longer run (bug fixes etc.).
• Approval from the co-leads - Johan or Kevin - to implement it.

One thing that caught my eye while I was looking at the D3D implementation a while back was that the point light implementation is a custom one and not the point light given by the D3D API. I don't know why that is, maybe to unify the behavior across platforms. I'll re-ask about releasing Prism internal documentation.

In any case, if you intend to work on OpenJFX you'll need to sign the OCA if you haven't done so already.

[nlisker]

I asked for the Prism documentations, but it's not guaranteed we'll get any. Meanwhile, I'll do some experimentation in d3d with the point light and see what kind of mayhem it causes. If you can try to meddle with the OpenGL pipline we'll get an idea of what we are up against.

[ProggerFox]

Evening,

sorry for my long absence, job and building up the working environment.
I prepared most of the steps you mentioned, this i will shortly open a new thread to ask for more help.
About the guarantee you asked for, could you elaborate more on that? And what would be the terms, because of course it is in my main interest to improve and stabilize this framework (since i also use it mainly for my projects), but i can not guarantee that i will always be available.

Also i sent the OCA to Oracle, approval stil needs to be given, after that i will public something here so you can check out my findings.
Regarding this i took some researchs into the ES2 pipeline and it seems that there might be a need for additional shaders or at least slights modifications to existings shaders to implement the attenuation factors.
There are even Lights-functions in OpenGL directly use the underlying PointLight (also Directional/Spot-Lights too), it might be enough to just use these and if not, then the old way of plainly adding and modifying values it is then.
No big deal in the end, the bridge for Prism-JavaFX might need more work, but definitely doable.
I will start with this after i finished the mentioned steps and got the approval.

Else i am working on this case, just to let you know.

Cheers!

[ProggerFox]

Regarding the Apple's pipeline-change, we should try to settle this as fast as well, since it will surely affect how we should tackle our own matters.
This we should further discuss in the corresponding issue, though.

To add to that i would tend to Vulkan + MoltenVK, simply for the reason that by going this route we ensure a higher portability.
Also they provide a specific solution to enable the Vulkan-pipeline on Macs, via converting from Vulkan to Metal shaders, thus i doubt that there will be such a big performance hit in the end.
This is at least from my perspective, in the end we should of course try to go with the route the community decides.

[nlisker]

About the guarantee you asked for, could you elaborate more on that?

Not really. It's not a formal agreement. One of the things the co-leads evaluate when we ask for permission to implement this is if someone will be able to maintain it. Otherwise, people will add their code and leave, leaving technical debt. If you plan to stay around (I certainly do) I guess it will be fine, but they will decide.

Also i sent the OCA to Oracle

Good. It will take some time to be approved, but we can work meanwhile. It just stops you from submitting a PR/patch.

Regarding this i took some researchs into the ES2 pipeline and it seems that there might be a need for additional shaders or at least slights modifications to existings shaders to implement the attenuation factors.
There are even Lights-functions in OpenGL directly use the underlying PointLight (also Directional/Spot-Lights too), it might be enough to just use these and if not, then the old way of plainly adding and modifying values it is then.

If I understand you correctly, the ES2 pipeline defines a custom point light instead of using the OpenGL functions? That's the case for D3D. The thing is, the material (I guess what you call shader) and lighting are coupled, so if I use the D3D PointLight implementation it won't affect the material. Is this what you mean?
So I think we have options:

1. Mimic the D3D/OpenGL lights by expanding the current implementation.
2. Replace the current implementation with the libraries implementation.

Not sure which is worse :) The first one is more flexible and we might avoid inconsistencies between the pipelines, the second one has ready-to-use implementations but seems more ambitious.

Regarding the Apple's pipeline-change, we should try to settle this as fast as well, since it will surely affect how we should tackle our own matters.
This we should further discuss in the corresponding issue, though.
...

Yeah, the graphics discussion has been brought up several times in the past, it will probably be brought up again when we start this discussion, and I'm guessing it will be brought up again after the work is done...

#193
http://mail.openjdk.java.net/pipermail/openjfx-discuss/2018-October/000025.html
http://mail.openjdk.java.net/pipermail/openjfx-discuss/2018-October/000038.html
http://mail.openjdk.java.net/pipermail/openjfx-discuss/2018-October/000001.html
http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-June/021965.html
More discussion from February if you want to dig...

No big deal in the end, the bridge for Prism-JavaFX might need more work, but definitely doable.
I will start with this after i finished the mentioned steps and got the approval.

That's great. This week might be loaded for me but I'll try to squeeze in a research at an adequate level to get a better idea of what needs to be done on D3D.

[ProggerFox]

Not really. It's not a formal agreement. One of the things the co-leads evaluate when we ask for permission to implement this is if someone will be able to maintain it. …

Then this wont be much of a problem.
It is not in my intention to leave this issue unfinished, so there is my guarantee ;)

If I understand you correctly, the ES2 pipeline defines a custom point light instead of using the OpenGL functions?

ES2 surely uses the regular OpenGL light-sources, there should be no custom implementation of any lightings, as far as i know.
At the moment there are Light-structs created in the shaders, which simply contain color- and location-vectors, which are further on used in the calculation for the final result (overall illumination of (a point of) the target)
These values will be used later on in creating said lightings in the renderer.

But i proposed, that we could utilize the said "native OpenGL"-lightings via the shader (i recall it was possible to create light-sources in that way too) and thus "just" use the native lightings in OpenGL.
So instead of implementing own mathematics and calculations, we do a kind of "executive"-approach, by telling at which point which kind of lighting should be created/used.
Since OpenGL already is able to produce up to 8 different kind of lightings (Spot, Directional, Point, Ambient, etc..) by setting certain parameters, we might shift a little of the work for different types of lightings in ES2 at least.
But i dont know if this is truly applicable with the overall structure of JFX, that i will have to test yet (also i never did it that way before too).
Here is the proposed light-function in OpenGL by the way:
https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/glLight.xml

The thing is, the material (I guess what you call shader) and lighting are coupled, so if I use the D3D PointLight implementation it won't affect the material. Is this what you mean?

The materials are generally affected by the lights in a 3d world, if i am not mistaken.
In the case of lights it is mostly the brightness and sometimes also the color value that affect the resulting material in overall.
But how is this realized in D3D then, if it doesnt affect the material?
In OpenGL the lights add to the resulting color of a material and depending on the light, this is at a certain spot only.

And a shader is basically a "little program" that is given to the GPU to execute (graphical) effects/results.
In fact, it is just a plain C-file (except it normally has the extensions .frag, .vert, etc... depending on what kind of shader it is) where one basically modify values, mostly by doing heavy mathematics, to achieve the corresponding values to produce said results.
To say the shader is not specifically the material, but it is the "producer" of said materials/ lightings, etc...
In a OpenGL-app (or in subsets) one has generally a set of different shaders to execute specific effects (or a combination of said shaders)
You can find the said shaders for ES2-JFX here, if you want to take a look into the existing shaders for yourself:
https://github.com/javafxports/openjdk-jfx/tree/develop/modules/javafx.graphics/src/main/resources/com/sun/prism/es2/glsl

And what i meant was that ultimately the shaders respsonsible for the lightning lack the attenuation factors in the calculation for the final output, so they either have to be added to the corresponding shaders or we add new shaders that extends said functionalities (unlikely and unfavoured for the moment)
I would tend towards the first option though, simply because all lights will make use of attenuation factors, thus it would make sense to implement it as a base functionality for all light-processings.
But then, as previously stated, i suggested the approach of using the native lights, if possible.
So tinkering with the mathematics in the shaders would be the solution after the first one would fail, thats my plan at least.

So I think we have options:
Mimic the D3D/OpenGL lights by expanding the current implementation.
Replace the current implementation with the libraries implementation.
Not sure which is worse :) The first one is more flexible and we might avoid inconsistencies between the pipelines, the second one has ready-to-use implementations but seems more ambitious

I cannot speak for D3D, but for ES2 it should be enough to be able to retrieve all the necessary datas to set up its attenuations factors, the rest of it will be done via the said shaders.
Not to speak of it as a simple matter, but i suspect a "simple bridge" to be able to set attenuation factor values via JFX and a functionality for retrieval from Prism will suffice for ES2 in the end.
So yes, if you mean by mimic the pipelines that we will use the pipeline-structures as reference to further expand the Prism/JavaFX-implementations, then yes, this is exactly what i was hoping for how it would turn out :)
Many of the functionalities, that seems to be lacking in JFX, already have industry- standard implementations that can be found everywhere around in the web (in fact i found an industry-standard for implementing attenuation factors in OpenGL already)
So hopefully the biggest load of work will be maintaining the Prism-layer in the end, and for it to mainly function as a data-provider for all of the pipelines, which in turn can be set on the JFX-side.

How exactly would you go on about working on your said options though?

Yeah, the graphics discussion has been brought up several times in the past, it will probably be brought up again when we start this discussion, and I'm guessing it will be brought up again after the work is done

The suggested route of the switch to ANGLE seems quite interesting... (http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-June/021973.html)
We could also look into how to switch to another render-pipeline that is already maintained and provides a lot of functionality many of the JFX-devs seem to need.
This wont change our work though, and for the moment we should continue with Prism until a clear state is reached.
An issue regarding that should be opened then.

That's great. This week might be loaded for me but I'll try to squeeze in a research at an adequate level to get a better idea of what needs to be done on D3D

Same here, time is not benefical to me either.
But i already found somebody helping me out with working on the ES2-pipeline, so it wont be much of a problem.
Looking forward to your findings.

[nlisker]

Just updating that after I discussed this with Ambarish (one of the Oracle devs), he gave me some reading material, so I'm doing the reading now.

[nlisker]

Iv'e managed to implement attenuation. It's very preliminary, but as a POC it works. The pictures show a box's face with a light source in front, and I move the box farther away each time.

Did you have any luck on the OpenGL side?

[ProggerFox]

Happy new year!
And wow, there i decide to check this forum out after a long while and one gets a very pleasant surprise for the new year too ;)
Great job!
How did you accomplish your solution in D3D?

On my side, I worked out some modifications for the shaders, which was nothing great to be honest.
The Light-struct was simply extended with further parameters, a calculation to get the light intensity over a distance pre-returning the light was added to the corresponding fragment-shaders and missing uniforms were added to the render-instance.
Else most of the functionality was already provided, but apparently not utilized.
Though i had a slight hangup on the general "Bridge" for Prism at one point, for which i wanted to come up with a common solution with you before continuing further.
Sadly i recently have very little time on my hand, due to other projects and private matters, so progress wont happen as fast as expected.

Anyway, if possible i would like to modify my solution to be compatible with yours, if needed.
It is great that at least one of us is able to provide a display of progress, this may encourage others to use JavaFX for 3d-applications even more already :)

Also could you provide me the said reading material you got as well? Thanks!

[nlisker]

How did you accomplish your solution in D3D?

Not sure on what level you are talking about. On the shader level, I passed the world coordinates from the vertex shader to the pixel shader. Then I mocked a light position and calculated the distance between them and divided the diffuse color value by the distance (up to a constant). I didn't do anything outside of the Prism 3D3 graphics pipeline yet as I was working towards a POC only.

The solution requires to upgrade the shader models from 2.0 to 3.0, which is parallel to upgrading DirectX from 9.0 to 9.0c. This will require authorization by Kevin/Johan as it's a backwards compatibility issue.

Though i had a slight hangup on the general "Bridge" for Prism at one point, for which i wanted to come up with a common solution with you before continuing further.

Definitely. We need to think about the end result first. Attenuation is used only by some light types (https://docs.microsoft.com/en-us/windows/desktop/direct3d9/light-types):

• Ambient
• Directional
• Point
• Spot

So only they should have them. We will need 3 attenuation parameters (https://docs.microsoft.com/en-us/windows/desktop/direct3d9/attenuation-and-spotlight-factor) and a range parameter which is a hard cutoff for attenuation calculation. These will be DoublePropertys.

The mathematical behavior for each light should be (in pseudocode):

float attenuation;
float dist = distance(pixelPos, lightPos);
if (dist > range) {
    continue to next light; // same as attenuation = 0: light does not participate
} else {
        attenuation = 1/( a0 + a1 * dist  + a2 * dist ²);
}
float[3] diffuseColor = diffuseColor * attenuation;
float[3] specularColor = specularColor * attenuation;

Where the last 2 lines mean that the attenuation factor multiplies whatever the calculation was before. Optimizations not included.

As for the Java side, I can take care of it, or at least most of it. Last time I looked I didn't foresee any problem there.

Also could you provide me the said reading material you got as well? Thanks!

It's a collection of some very general to specific HLSL related links:

• https://stackoverflow.com/questions/38938498/how-do-i-convert-gl-fragcoord-to-a-world-space-point-in-a-fragment-shader
• http://developer.download.nvidia.com/CgTutorial/cg_tutorial_chapter01.html
• https://docs.microsoft.com/en-us/windows/desktop/direct3dhlsl/dx-graphics-hlsl-writing-shaders-9#uniform-shader-inputs
• https://docs.microsoft.com/en-us/windows/desktop/direct3dhlsl/dx-graphics-hlsl-semantics#migration-from-direct3d-9-to-direct3d-10-and-later
• https://docs.microsoft.com/en-us/windows/desktop/direct3d9/texture-coordinates

I'm going to start a discussion in [email protected], please follow it and chime in when needed.

[nlisker]

Some experimentation with SpotLight implementation:

The red dot is the light source location. The light direction is +z (directed at the surface directly). The inner angle is 30 deg, below which the surface receives maximum intensity; the outer angle is 60 deg, above which the surface receives no light; the part between them is governed by a quadratic falloff using the formula given by Direct3D.

Here the light direction is (1, 0, 1) (45 deg between +x and +z).

Distance attenuation was not included for simplicity.

[ProggerFox]

How did you accomplish your solution in D3D?
Not sure on what level you are talking about.

I was interested in how you realized your POC, since you already established a way to render JavaFX property-changes to the D3D-renderer.
From what i know after i looked into the sources for the D3D-pipeline there seems to be a similar structure in the rendering-workflow, thus at some previous point there might be an opportunity to merge both of our solutions.
By maintaining a complementary structure we would save later hassle, while also ensuring a possible interface for the other render-pipelines.
TL;DR: How did you realized the "data-transfer" from Java-to-D3D or vice-versa? (on a concrete level)
Since our takes on the shader level are pretty much similar, where i also practically just modified the shader-logic, i assumed we could try to merge the functionality of processing attenuations factors and have a base for the others to work with already.

Definitely. We need to think about the end result first. Attenuation is used only by some light types (https://docs.microsoft.com/en-us/windows/desktop/direct3d9/light-types).
So only they should have them. We will need 3 attenuation parameters (https://docs.microsoft.com/en-us/windows/desktop/direct3d9/attenuation-and-spotlight-factor) and a range parameter which is a hard cutoff for attenuation calculation. These will be DoublePropertys.

Sounds like a plan, i agree with it.
Not sure why but i found references for using attenuation for ambient light too, for whatever reason this was done.
Maybe it can be kept open to all light types to use attenuation at a later point, if necessary.
We will stick with point and spot light for the moment though.

The mathematics used in your shader seems solid, though with non-included optimizations: are you hinting to gamma issues by chance?

As for the Java side, I can take care of it, or at least most of it. Last time I looked I didn't foresee any problem there.

Fine by me.
I will prepare something on my side too in a hopefully shorter time than lately.

I'm going to start a discussion in [email protected], please follow it and chime in when needed.

Long due after my long absence. Gladly!

[ProggerFox]

Some experimentation with SpotLight implementation

Already on the next light type. One is having fun here as i see :P
Though on the last image the light looks a little confusing, it looks like the light source itself is some kind of projectile flying with high speed.
Also maybe it is the angle of the picture taken, but isn't the light emitting somewhat prior to the light sources location? (except there is some kind of light reflection going on)
Just my 2 cents, but else, good job!

[nlisker]

I was interested in how you realized your POC, since you already established a way to render JavaFX property-changes to the D3D-renderer.

Actually I didn't tie it to the Java side yet, but that's not difficult. The attenuation and range numbers are passed to the native code just like all the other parameters such as light position and color.

From what i know after i looked into the sources for the D3D-pipeline there seems to be a similar structure in the rendering-workflow, thus at some previous point there might be an opportunity to merge both of our solutions.
By maintaining a complementary structure we would save later hassle, while also ensuring a possible interface for the other render-pipelines.

Yes, the Java side is the same up to some point near the native calls. That part I'll do. When it diverges we might not be able to use a complementary structure because the native level could be very different, will have to look.

Since our takes on the shader level are pretty much similar, where i also practically just modified the shader-logic, i assumed we could try to merge the functionality of processing attenuations factors and have a base for the others to work with already.

Not sure what is "the functionality of processing attenuation factors". The attenuation factors are passed as floats to the native level and only there processed.

Not sure why but i found references for using attenuation for ambient light too, for whatever reason this was done.
Maybe it can be kept open to all light types to use attenuation at a later point, if necessary.

How can you attenuate a light that has no position? What is the reference?

The mathematics used in your shader seems solid, though with non-included optimizations: are you hinting to gamma issues by chance?

I looked at gamma correction, but didn't think of implementing it yet. We could look at it.
"Optimizations not included" means that the code might not be optimized for a shader, it's just to show the point.

Also maybe it is the angle of the picture taken, but isn't the light emitting somewhat prior to the light sources location?

It's the angle. I do have a problem, though, with not illuminating other faces of the cube with this light, which could be a vertex shader issue.

I'm going to start a discussion in [email protected], please follow it and chime in when needed.

The discussion is at: http://mail.openjdk.java.net/pipermail/openjfx-discuss/2019-January/000092.html

[ProggerFox]

Yes, the Java side is the same up to some point near the native calls. That part I'll do. When it diverges we might not be able to use a complementary structure because the native level could be very different, will have to look.

Alright, i will leave the Java side to you then. From my side the ES2 shader-stuff is ready to be used. About to prepare some nice pictures of it too ;)

Not sure what is "the functionality of processing attenuation factors". The attenuation factors are passed as floats to the native level and only there processed.

Skip what i said, i needlessly foresaw conflicts where there might not be some.

How can you attenuate a light that has no position? What is the reference?

I think it was used as a way to change the general brightness if i recall correctly, so it acted kind of as a "light power" for ambient light. It is said that ambient light does in fact have a position, but one that isn't determinable anymore, since the light scattered off in all directions too many times (that's how it is stated at least)
The references to this were comments in certain forums, but also OpenGL allows to make combinations like with ambient-light and attenuation applied, thus resulting in varying brightness.
You can have a look here about how light types in OGL are realized, if you want: http://glprogramming.com/red/chapter05.html

But this can already be achieved by providing a darker color to the light in JFX and we can skip it, just found the usage case curious.
It might be an OGL-only-thing in the end anyway.

I looked at gamma correction, but didn't think of implementing it yet. We could look at it.
"Optimizations not included" means that the code might not be optimized for a shader, it's just to show the point.

Sure, i will help you out. Maybe the reference for how its done in OpenGL can help: https://www.tomdalling.com/blog/modern-opengl/07-more-lighting-ambient-specular-attenuation-gamma/
Else you could tell me where exactly the file is you are planning to work on (if not added as new one), so i will have a look myself.
And generally the code looks fine for the moment, nothing too big happening or any bottlenecks to be recognized yet.

It's the angle. I do have a problem, though, with not illuminating other faces of the cube with this light, which could be a vertex shader issue.

Not illuminating the other faces could have to do with the spot-light's ambient light component, since spot-light is supposed to use it partially.
In this regard, maybe "Ambient Reflection" could help you out more.
Also you can show me the source code, then i might take a look into it myself.

The discussion is at: http://mail.openjdk.java.net/pipermail/openjfx-discuss/2019-January/000092.html

Got it.

[nlisker]

We got the green light for attenuation factors and additional light types. We're starting with PointLight attenuation as discussed previously. Iv'e submitted JDK-8217472. There are 3 subtasks - I'll do 1 and 3, you are responsible for 2. You don't need to do anything in JIRA (as I said, the paperwork is on me), just open a pull request with the same name as the subtask and add a link to it. Your OCA should be approved by now, is it?

There's still some coordination we need to do regarding the transition between the joint Prism layer (task 3) and the Prism pipeline-specific layer (tasks 1 and 2). We also need to choose names for the properties, which differ between specifications.

[nlisker]

This is the experiment with range:

It provides a sharp cutoff, zeroing the effect of the light.

[nlisker]

I have my changesets ready. We need to decide on the exact public API, which is mostly the names of the new properties we're introducing. I plan to submit task 3 first, then I'll provide a test program (similar to the one in the images above) so we can make sure the effects look the same.

[ProggerFox]

Good day, had quite a long week.
The progress on your side looks great, good job on your work.

Your OCA should be approved by now, is it?
Regarding this i am a little confused about the OCA: I suppose i should appear on the list of signatories, but i cant find myself there. So i suppose i am not yet allowed to commit my works, no email either until now.
But i will look into undertaking the subtask anyway, thus let's just continue anyway.

I have my changesets ready. We need to decide on the exact public API, which is mostly the names of the new properties we're introducing. I plan to submit task 3 first, then I'll provide a test program (similar to the one in the images above) so we can make sure the effects look the same.

Sounds good, then we will just test if my changes work on your program as well.
As for the property names, i suppose the documentations we both used refer to similar names anyway, which were "Distance", "Constant(Attenuation)", "Linear(Attenuation)" and "Quadratic(Attenuation)", if i recalled them right.
Of course we could come up with more descriptive or "easier" names for the folk that aren't dealing much with the graphics pipeline as well.
So maybe we could also use:
Constant-Attenuation = Limit or Softness
Linear-Attenuation = Scattering or Absorption
Quadratic-Attenuation = Sharpness (as in harder cutoff?)
These would be my suggestions for the names.

[ProggerFox]

There's still some coordination we need to do regarding the transition between the joint Prism layer (task 3) and the Prism pipeline-specific layer (tasks 1 and 2). We also need to choose names for the properties, which differ between specifications.

What would be the other points we would have to clarify then?
Also i would like to know from you: In the ES2-pipeline i encountered a limitation for the possible number of supported Point-lights, thus is there something like that in D3D as well?
In the sources of ES2 it was stated that there wont be more than 3 point-lights supported for the illumination of a MeshView at that time being.
So practically a MeshView will start ignoring further light-sources, not sure why that is though.
But maybe i could just disregard and/or remove this limitation, since only 3 influencing light-sources at max seems quite limiting imho.

[nlisker]

@kevinrushforth We need some help here.

Regarding this i am a little confused about the OCA: I suppose i should appear on the list of signatories, but i cant find myself there. So i suppose i am not yet allowed to commit my works, no email either until now.

Kevin, can you look into this please?

As for the property names, i suppose the documentations we both used refer to similar names anyway,
which were "Distance", "Constant(Attenuation)", "Linear(Attenuation)" and "Quadratic(Attenuation)", if i recalled them right.
Of course we could come up with more descriptive or "easier" names for the folk that aren't dealing much with the graphics pipeline as well.
So maybe we could also use:
Constant-Attenuation = Limit or Softness
Linear-Attenuation = Scattering or Absorption
Quadratic-Attenuation = Sharpness (as in harder cutoff?)
These would be my suggestions for the names.

They have to conform to the Java naming conventions. So, we can do constantAttenuationFactor etc., but they seem too long. Maybe constantAttnFactor etc.
I also think that range is better than distance because the latter is the actual distance between the light and the mesh.

What would be the other points we would have to clarify then?

Iv'e added the required parameters to MeshView#setPointLight, which required me to change ES2MeshView to add the parameters to the implementing method. After I submit this patch, you will need to pass these parameters down the pipeline to new ES2Light etc. This is the only change I made to the es2 pipeline and it was done so the patch would compile.

Also i would like to know from you: In the ES2-pipeline i encountered a limitation for the possible number of supported Point-lights, thus is there something like that in D3D as well?

I think this was done because of a limitation of the D3D shader model. The number of variables that can be passed from the vertex shader to the pixel shader is limited, though it was capped at 3 where there was space for 1 more. After upgrading to Shader Model 3 we should be able to have 5. I could look into calculating the position of the lights on the pixel shader, which will allow dozens of lights, but I'm not sure about the performance change there.

[ProggerFox]

@kevinrushforth We need some help here.
Regarding this i am a little confused about the OCA: I suppose i should appear on the list of signatories, but i cant find myself there. So i suppose i am not yet allowed to commit my works, no email either until now.
Kevin, can you look into this please?

Found myself on the list today, seems everything is clear now.

They have to conform to the Java naming conventions. So, we can do constantAttenuationFactor etc., but they seem too long. Maybe constantAttnFactor etc.
I also think that range is better than distance because the latter is the actual distance between the light and the mesh.

Then how about just "constantAttenuation", etc.?
Since the suffix "Factor" might be self-explanatory, so we could as well leave these out.
Also agreed on range, it was my first guess too, albeit every factor sorts of influences the distance of the light from the source.
This is quite a jungle of definitions to be honest, thus in hindsight we should really stick to the names of the attenuation constants to avoid confusions.
A lot of online references (and later documentation) should be available for people to find by these names alone to do further Researchs as well.

Iv'e added the required parameters to MeshView#setPointLight, which required me to change ES2MeshView to add the parameters to the implementing method. After I submit this patch, you will need to pass these parameters down the pipeline to new ES2Light etc. This is the only change I made to the es2 pipeline and it was done so the patch would compile.

I see, so you practically did my work already :P
Kidding.
Alright, i will keep your changes in mind and will have a look at them then.
Did you modified the additional specifications for other platforms without errors as well? (I suppose it was only the specification for the windows platform, right?)
Are there more things we have to keep in mind?

I think this was done because of a limitation of the D3D shader model. The number of variables that can be passed from the vertex shader to the pixel shader is limited, though it was capped at 3 where there was space for 1 more. After upgrading to Shader Model 3 we should be able to have 5. I could look into calculating the position of the lights on the pixel shader, which will allow dozens of lights, but I'm not sure about the performance change there.

I remember you were addressing this issue on the JDK Bug System.
Quite frankly it will have an impact on the performance, since the position is calculated at the vertex-phase, which surely is optimized for that matter.
I am not sure how to help in this matter yet either though, maybe as a draft it would be acceptable to calculate the position on the pixel shader for the time being.
We will look more into this of course.

[nlisker]

Then how about just "constantAttenuation", etc.?
Since the suffix "Factor" might be self-explanatory, so we could as well leave these out.

That could work. I'll suggest an API (methods + their docs) in a followup comment. Once we approve it I'll submit a CSR.

Did you modified the additional specifications for other platforms without errors as well? (I suppose it was only the specification for the windows platform, right?)

Yes, the D3D and ES2 pipelines are the only ones that support 3D.

Are there more things we have to keep in mind?

We'll probably need to write some tests. We'll see which when we get the patches ready.

In the meantime, can you open a PR titled "[WIP] JDK-8217477: Add attenuation for the es2 pipeline" and put the link https://bugs.openjdk.java.net/browse/JDK-8217477 in the first comment? It doesn't have to compile yet, but it should once the connection between MeshView and ES2meshView is made.

Quite frankly it will have an impact on the performance, since the position is calculated at the vertex-phase, which surely is optimized for that matter.
I am not sure how to help in this matter yet either though, maybe as a draft it would be acceptable to calculate the position on the pixel shader for the time being.
We will look more into this of course.

That would be good. I'm not familiar enough with performance considerations. Though, are 4 or 5 lights per mesh not enough? Remember that it affects only meshes that are in the scope of the lights. And then we can do some crude optimization for which of the lights to use based on intensity. If a light is out of range of the bounds of the shape (or for spotlights, not in their direction) we can discard it in the Java level maybe.

[ProggerFox]

In the meantime, can you open a PR titled "[WIP] JDK-8217477: Add attenuation for the es2 pipeline" and put the link https://bugs.openjdk.java.net/browse/JDK-8217477 in the first comment? It doesn't have to compile yet, but it should once the connection between MeshView and ES2meshView is made.

Sure, i will try to tackle this matter at weekend, as i do with the other matters here on GitHub.
Also i already established a connection between the NGMeshView and the ES2MeshView, just i still have to validate a working result.
Thus i will include these changes in the soon to come PR as well, maybe there is a possibility to unify both our solutions if needed.

That would be good. I'm not familiar enough with performance considerations. Though, are 4 or 5 lights per mesh not enough? Remember that it affects only meshes that are in the scope of the lights. And then we can do some crude optimization for which of the lights to use based on intensity. If a light is out of range of the bounds of the shape (or for spotlights, not in their direction) we can discard it in the Java level maybe.

It depends a lot on the complexity of the scene, for not so big or complex sceneries 4 to 5 lights would suffice definitely.
But consider the scenario of a mesh being illuminated from all sides (right, left, front, back) and maybe from additional sources (flashlights, etc.) and you will very soon hit a limitation. A scenery like that could be a festival with a decent amount of lamps hanging around (albeit this technically would drain the performance by a hefty amount).
It was just a consideration, right now we both work with pretty outdated shader models, so we cant expect great technical features anyway.

Also surely we could move some of the work to the Java-side as well, but this surely will prove to be not very performant.
But we can discuss this later as well, right now the progress is doing very good so far

I try to prepare everything needed in a short (as possible) time, so that we can finally have the first presentable results ready for other to use.

[nlisker]

Here is my proposed API. It will be accompanied by an update to the class documentation.

/**
 * The range of this {@code PointLight}. A pixel is affected by this light i.f.f. its
 * distance to the light source is less than or equal to the light's range.
 * Any negative value will treated as {@code 0}.
 * <p>
 * Lower {@code range} values can give better performance as pixels outside the range of the light
 * will not require complex calculation. For a realistic effect, {@code range} should be set to
 * a distance at which the attenuation is close to {@code 0} as this will give a soft cutoff.
 * <p>
 * Nodes that are inside the light's range can still be excluded from the light's effect
 * by removing them from its {@link #getScope() scope}. If a node is known to always be
 * outside of the light's range, it is more performant to exclude it from its scope.
 *
 * @defaultValue {@code Double.POSITIVE_INFINITY}
 * @since 13
 */
public final DoubleProperty rangeProperty()

public final void setRange(double value)

public final double getRange()

/**
 * The constant attenuation coefficient. This is the term {@code c} in the attenuation formula:
 * <p>
 * <code>attn = 1 / (c + lc * dist + qc * dist^2)</code>
 *
 * @defaultValue {@code 1}
 * @since 13
 */
public final DoubleProperty constantAttenuationProperty()

public final void setConstantAttenuation(double value)

public final double getConstantAttenuation()

And similarly for linearAttenuation and quadraticAttenuation with default values 0.

[kevinrushforth]

I think this is a good start at the API for this, but need to review in more detail. I have a couple comments:

1. When you file the CSR please indicate that these are being added to the PointLight class.
2. You need to add the @since 13 javadoc tag.
3. I know that Microsoft uses range to mean the cutoff distance for attenuation, but I wonder if cuttoff might be a better name (typically range has both a min and a max value, such as IndexRange, PageRange, and AxisRange classes). Although cutoff might be confused with cutoffAngle which we will want when adding a SplotLight class.

[nlisker]

I added @since 13 in an edit probably as you were writing.

Point 3 is interesting. I would say:

• All these examples are for the controls module, and we are in graphics.
• We could add a minRange and maxRange, although having a minRange for a light is a bit funny.
• SplotLight will have innerAngle and outerAngle as far as I planned, so cutoff is available.

So I see reasons for range, maxRange and cutoff all being viable.

[nlisker]

@FalcoTheBold Do you have any update?

@kevinrushforth Can we continue with the review?

[ProggerFox]

@nlisker On it to upload content soon.
Work is keeping me quite busy lately, sorry for the delays.

[ProggerFox]

@nlisker Alright, i built up a seemingly working flow from the JavaFX-side to the ES2 pipeline, albeit for the PR i will leave these JavaFX-parts out.
We should start to talk about how to implement these features on the FX-side in a way that both our pipelines will work accordingly.

In order for a PoC i modified the java/javafx/scene/LightBase-class, by adding the 4 properties with the previously discussed named convention (range, constantAttenuation, etc.)
These properties are then set in the corresponding java/com/sun/javafx/sg/prism/NGLightBase which are at last used in the java/com/sun/javafx/sg/prism/NGShape3D.
From then on it translates into all of the pipeline-workflow, which i have set up for ES2 by now.
Here are the modified classes required on the JavaFX side for you to take a look in, i mostly marked all the places that were changed with my name (FalcoTheBold)
ccc.zip
Let me know if this is something you can work with.
Also please give me intel about the default values for each one of the 4 properties again.

I will commit the ES2 changes to the PR later when i am back at home.

[ProggerFox]

@nlisker I created the PR now, though i made it a draft for the moment since smaller fixes need to be done yet.
Also for the sake of completeness i added the code for the Java-side too.

[ProggerFox]

@nlisker At a later point today i will add the remaining Fragment-shader files, had them on another location and can upload them only after i got them from there.
Anyway, i have a lack of understanding how to process the attenuation factors in the case of multiple lights, while at the moment i just multiply each attenuation calculation result and then apply them to the final diffuse + specular components.
Would that be correct?

[nlisker]

@FalcoTheBold

Here are the modified classes required on the JavaFX side for you to take a look in, i mostly marked all the places that were changed with my name (FalcoTheBold)
[ccc.zip](https://github.com/javafxports/openjdk-jfx/files/2884708/ccc.zip)
Let me know if this is something you can work with.

These changes are part of the Java layer, which I'm taking care of. I understand you need to make those too so you could test your changes in the native layer. I should have posted my changes so we could have the same framework to work with, it needs to go into review first anyway and it's been sitting waiting for the docs review, but we don't need the docs for this.

In any case, your changes look very similar to mine, so that's good.
One thing you should be careful of are negative values, which should be treated as 0 (though it's interesting to have negative attenuation, as it allows to create a "shadow caster" instead of a "light caster" 🤔).
Another difference is that I added the attenuation properties to PointLight and not LightBase because not all lights use them. Only PointLight and SpotLight will. Additionally, a direction property will be added to DirectionalLight and SpotLight, which is not used by the others. SpotLight will also have the inner and outer angles which only it uses. So, each light will have what it uses. Since we can't do any implements/extends on current code, I think that this is our best option. I don't think having subclasses ignore properties of their superclass is good design.

Also please give me intel about the default values for each one of the 4 properties again.

This is also part of the Java layer. They are chose to the attenuation is 1 to preserve backwards compatibility:

range = Double.POSITIVE_INFINITY 
a0 (constant) = 1.0 
a1 (linear) = a2 (quadratic) = 0.0

At a later point today i will add the remaining Fragment-shader files, had them on another location and can upload them only after i got them from there.

Good. This is the important part since this is where the actual visual behavior is and we need to make sure our implementations produce identical results.

Anyway, i have a lack of understanding how to process the attenuation factors in the case of multiple lights, while at the moment i just multiply each attenuation calculation result and then apply them to the final diffuse + specular components.
Would that be correct?

I think so. The calculation is done for each light: attenuation * lightColor * "geometric effect (angle)", and add this to diffuse/specular color. Here is the HLSL code:

 for (int i = _s; i < _e; i++) {  // loop over the number of lights
     float range = gLightAttenuation[i].w; // range is stored as the 4th component of the attenuation
     float dist = length(L[i].xyz); // L[i].xyz is the light position
     if (dist <= range) {
         float c = gLightAttenuation[i].x;
         float lc = gLightAttenuation[i].y;
         float qc = gLightAttenuation[i].z;
         float attn = 1.0 / (c + lc * dist + qc * dist * dist);

         float3 l = normalize(L[i].xyz);
         d += saturate(dot(n, l)) * gLightColor[i].xyz * attn; // d is the current diffuse color, each light adds to it
         s += pow(saturate(dot(-refl, l)), power) * gLightColor[i].xyz * attn; // same for specular
     }
 }

[kevinrushforth]

@kevinrushforth Can we continue with the review?

I presume you mean for the API (since the implementation is still WIP). Yes, that seems fine.

Point 3 is interesting. I would say:

• All these examples are for the controls module, and we are in graphics.
• We could add a minRange and maxRange, although having a minRange for a light is a bit funny.
• SplotLight will have innerAngle and outerAngle as far as I planned, so cutoff is available.

In that case, I prefer cutoff since it seems odd to have an attribute named range be a single limit value.

Once we have agreement on this, then the specification section of the CSR can be updated to indicate that the changes are for the PointLight class, and to format it using markdown formatting.

[ProggerFox]

@nlisker

I understand you need to make those too so you could test your changes in the native layer.

Exactly, or at least to make sure i get a compilable result for the PR (which i havent yet, as seen in the PR)
In order to make the AppVeyor build successfully i will at least implement a little bit more on the D3D-side this weekend.
But it would be nice if you would share me your code as well, since we seem to have a similiar solution it would be no problem to finally merge on the Java-side for the moment.
We adjust then towards the review requirements accordingly.
I made a fork of the openjfx-repo, if you want you can try to merge your results there and i make it fit with my solution.

One thing you should be careful of are negative values, which should be treated as 0 (though it's interesting to have negative attenuation, as it allows to create a "shadow caster" instead of a "light caster" 🤔).

Interesting, if by any chance the JavaFX-graphical quality would benefit thanks to that then i see no reasons not to consider such aspects 😀
But alright, i will make a validation check on that part then too.

Another difference is that I added the attenuation properties to PointLight and not LightBase because not all lights use them. Only PointLight and SpotLight will. Additionally, a direction property will be added to DirectionalLight and SpotLight, which is not used by the others. SpotLight will also have the inner and outer angles which only it uses. So, each light will have what it uses. Since we can't do any implements/extends on current code, I think that this is our best option. I don't think having subclasses ignore properties of their superclass is good design.

Yeah, you are right, there i was thinking a little too high for this case.
Will fix that in the upcoming commit as well 👍

This is also part of the Java layer. They are chose to the attenuation is 1 to preserve backwards compatibility:
range = Double.POSITIVE_INFINITY
a0 (constant) = 1.0
a1 (linear) = a2 (quadratic) = 0.0
At a later point today i will add the remaining Fragment-shader files, had them on another location and can upload them only after i got them from there.
Good. This is the important part since this is where the actual visual behavior is and we need to make sure our implementations produce identical results.

All done, default values were changed and the shader-files added, albeit i am sure they will have to be edited again at a later point, since i took the same approach of calculating the position in the fragment-shader for the moment.
But in regard for efficiency these files definitely need slight reworks, they dont look that well optimized to me.

I think so. The calculation is done for each light: attenuation * lightColor * "geometric effect (angle)", and add this to diffuse/specular color. Here is the HLSL code:
for (int i = _s; i < _e; i++) { // loop over the number of lights
float range = gLightAttenuation[i].w; // range is stored as the 4th component of the attenuation
float dist = length(L[i].xyz); // L[i].xyz is the light position
if (dist <= range) {
float c = gLightAttenuation[i].x;
float lc = gLightAttenuation[i].y;
float qc = gLightAttenuation[i].z;
float attn = 1.0 / (c + lc * dist + qc * dist * dist);

     float3 l = normalize(L[i].xyz);
     d += saturate(dot(n, l)) * gLightColor[i].xyz * attn; // d is the current diffuse color, each light adds to it
     s += pow(saturate(dot(-refl, l)), power) * gLightColor[i].xyz * attn; // same for specular
 }

}

Alright, thanks 👍
This will help me in the upcoming weekend.

[nlisker]

Sorry for the late reply, I've been busy recently.

@kevinrushforth

In that case, I prefer cutoff since it seems odd to have an attribute named range be a single limit value.

Once we have agreement on this, then the specification section of the CSR can be updated to indicate that the changes are for the PointLight class, and to format it using markdown formatting.

I've started to lean towards maxRange to allow for a future minRange if that becomes something we would like to do. It doesn't exist in the OpenGL or D3D specs, but we're not following them 1 to 1 anyways. We can also use farCutoff and nearCutoff similar to the camera clip planes. Although a "minRange"'s usage might not be obvious at this point, I think we shouldn't make it harder on ourselves to add on in the future because of a name choice.

@FalcoTheBold

Please find here the patch files for the shared Java layer. There is a git diff patch file, or inside the Webrev you can find the rt.patch file and browse the changes. Whichever works for you. I remind you that the ES2MeshView::setPointLight will require passing on the new parameters I made it receive.

Also in that folder you will find the package "lighting test" which includes a test program consisting of 3 java files. Just compile and run them with the modified OpenJFX. The application will show a box and a light source as shown in the images above (the light source starts inside the box, so move the box back). Mouse drag pans the camera and holding control while dragging will rotate it. The rest you modify with the sliders or text fields. It's not pretty but it's functional.

We will probably want to decide on some critical values and post what each sees using these values. though it would be best if someone has 2 machines side by side for comparison.

[kevinrushforth]

I've started to lean towards maxRange to allow for a future minRange if that becomes something we would like to do.

That's fine, too.

[nlisker]

@kevinrushforth Iv'e put the CSR up for proposal.

[ProggerFox]

@nlisker Finally managed a working build for the pull request. I hope it is okay for you that i included your code for the Java-side as well, it wasn't building else for me.

Please find here the patch files for the shared Java layer. There is a git diff patch file, or inside the Webrev you can find the rt.patch file and browse the changes. Whichever works for you. I remind you that the ES2MeshView::setPointLight will require passing on the new parameters I made it receive.

Also thanks for the sources again. For the ES2MeshView, no problem, my old setup handled your changes without flaws.
By the way, in the AppVeyor-build i noticed a lot of warning messages about HLSL not working with pow for negative values. It suggests you to use abs instead, but i suppose you are also planning to do further researchs regarding the "shadow caster"?

We will probably want to decide on some critical values and post what each sees using these values. though it would be best if someone has 2 machines side by side for comparison.

I can provide that, i have both systems at my disposal (one lacking a monitor at the moment though).
What critical values would you have in mind?

[nlisker]

I hope it is okay for you that i included your code for the Java-side as well, it wasn't building else for me.

Yes, the Java side code will be submitted first anyway, so only after it is merged will you need to finalize your code.

By the way, in the AppVeyor-build i noticed a lot of warning messages about HLSL not working with pow for negative values. It suggests you to use abs instead, but i suppose you are also planning to do further researchs regarding the "shadow caster"?

The pow calculation is not my code, it's for the specular power. The warnings were always there. The shadow caster is unrelated, and is build in just by virtue of not limiting the attenuation factors to be >=0. This is because the light's contribution to the diffuse color is (n.l) * color * attenuation where n.l is the scalar product of the normal and light vector where negative values are treated as 0. attenuation is the only part that can be negative, which makes the contribution negative, so that color is subtracted from the current diffuse and specular colors.

So if the box has color (0.5, 0, 0) and we shine a light of color (0.5, 0, 0), if the attenuation is -1 (let's say ca = -1, la = 0, qa = 0), the resulting color will be (0, 0, 0).

I can provide that, i have both systems at my disposal (one lacking a monitor at the moment though).
What critical values would you have in mind?

I didn't have specific numbers in mind, but each property should be tested independently. So la = 0 and qa = 0, and run ca over several values from -infinity to +infinity. Then keep ca = 1 and do the same for la and qa.

For this comparison I will have to give you the d3d patch as well.

[kevinrushforth]

I wanted to comment on one point:

Yes, the Java side code will be submitted first anyway, so only after it is merged will you need to finalize your code.

No, this needs to be done as a single unified contribution. I don't want to see the API and pipeline-independent implementation go in without the specific implementations for both D3D and ES2. Otherwise we would be committing a partial feature. In addition to that not being something we typically do, until we have solid implementations for both rendering pipelines we can't be sure that the common code won't require changes.

[kevinrushforth]

For similar reasons, the CSR should not be finalized until after the code review is mostly done (it's in good enough shape for the "Proposed" state, but it is likely that the code review might cause some changes).

[nlisker]

No, this needs to be done as a single unified contribution.

OK, so one of us will need to send their code to the other and submit it as a single patch?

[nlisker]

@FalcoTheBold

The Java + D3D patch is here. Combine it with your ES2 changes and we should have the full first version of the patch.

[kevinrushforth]

@nlisker @FalcoTheBold - Yes, a combined patch will be needed. And both of you will be listed as contributors for the patch. A pull request might be the easiest.

[ProggerFox]

@nlisker @kevinrushforth The combined changes are in the PR #384.
I suppose i will open the PR for the review after i did some testing (or should it be open for the review now?)

[kevinrushforth]

You can start a formal review whenever you feel it is ready. It will likely take a bit of time to complete it.

Prior to doing so, please fetch/merge the latest changes from the upstream develop branch. I might also recommend squashing the commits into a single commit and then force-pushing to make a clean slate for reviewers, but that's just a suggestion (after starting the review, please do not force push).

[nlisker]

I'm starting the preliminary review on the PR.

[ProggerFox]

@nlisker Morning.
Just wanted to let you know that i was away from my workplace for a while and busy with other projects as well, but i will soon continue with the testing and fixing.
Regarding testing with your program, i wasnt able to test on my homely workplace yet but prepared the program in a way that i hopefully am able to just run it on the Linux-platform, when i am back at home.
I will let you know the results then.

[nlisker]

Sounds good. What was the issue with testing in your environment? Will you be able to compare both pipelines?

We should remember that if we want to add the other light types to the next version (13), and whatever tests that are required, we will need to move slightly faster. Around early July is the deadline.

[ProggerFox]

Sounds good. What was the issue with testing in your environment? Will you be able to compare both pipelines?

The issue was: i wasn't at home 😄
Well , i will first have to set up OpenJFX on the Linux-platform later and then i guess i should be able to do the comparison, also see how my changes are rendered.
The rest depends on if i am able to not cause a mayhem of bugs 😉

We should remember that if we want to add the other light types to the next version (13), and whatever tests that are required, we will need to move slightly faster. Around early July is the deadline.

Sure, seems doable. The foundational work is done, we know by your tests that these changes are possible now, so nothing will stop us in that regard..

[nlisker]

@FalcoTheBold I filed #440. I would like you to do the GL pipeline (I'll show you my d3d changes if it helps). That enhancement is not related to this one.