WIP: JDK-8217477: Implement attenuation-coefficients for PointLight #384

ProggerFox · 2019-02-20T22:38:24Z

Adding attenuation factors to the ES2-pipeline of the JavaFX 3D-API

Adding attenuation factors to the JavaFX 3D-API

Added SetRange opeator

brcolow · 2019-02-21T15:35:25Z

I'm not sure if this is super WIP or not but it's pretty non-standard to add comments to all the changes with your handle "FalcoTheBold" as that's what git/hg history is for. Not a big deal just pointing it out this superficial (non-code related) detail.

- Added missing shader-files - Changed default values of attenuation-factors

ProggerFox · 2019-02-21T18:07:02Z

@brcolow They can indeed be considered super WIP, these comments are just reminders from the time when i worked on the code apart from my usual work station and are supposed to be removed/replaced later of course.

- added missing members for native MeshView-struct - changed 'la' to correct data-type in native setPointLight-operation

This reverts commit 3556ec7.

This reverts commit 18e58fe.

This reverts commit 0a5d380.

This reverts commit 444cc94.

nlisker

OpenJFX uses spaces in stead of tabs, please change this.

Moreover, there are many spacing changes, such as in D3DContext.java and I'm not sure at all these are within the style conventions of OpenJFX. Kevin would have to comment on this I think.

The comments with your name and what you changed might be useful for reviewers, just remember to remove them later.

nlisker · 2019-03-11T13:19:35Z

.idea/misc.xml

  <component name="IdProvider" IDEtalkID="9BC0DE4049720C1246C2D3B835C6F7C3" />
-  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_8" assert-keyword="true" jdk-15="true" project-jdk-name="1.8" project-jdk-type="JavaSDK">
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_8" default="false" project-jdk-name="1.8" project-jdk-type="JavaSDK">


I don't think changes to IDE files belong in this patch. Is it necessary?

No, these were set by my IDE when i imported OpenJFX it seems, but i changed these back already. Thus it will be resetted within the next commits.

nlisker · 2019-03-11T13:30:48Z

modules/javafx.graphics/src/main/resources/com/sun/prism/es2/glsl/main1Light.frag

@@ -82,8 +85,12 @@ void main()
    vec3 l = normalize(lightTangentSpacePositions[0].xyz);
    d = clamp(dot(n,l), 0.0, 1.0)*(lights[0].color).rgb;
    s = pow(clamp(dot(-refl, l), 0.0, 1.0), power)*lights[0].color.rgb;
+
+	//FalcoTheBold - added attenuation calculation for a single light
+	float dist = (lights[0].pos.xyz - gl_Position) / lights[0].atten.range;


I don't understand this. If the distance from the pixel to the light is lights[0].pos.xyz - gl_Position, why divide by the range? If that distance is larger than the range, the light should have no effect. I don't see how this happens.

Honestly, tiredness got the better of me that time and i was led too much by findings in other forums regarding how to use the range in these places. Truly an error on my part there. There will be a conditional element regarding the affected range instead in the next commit as well.

nlisker · 2019-03-11T13:37:44Z

Did you test with the test code I linked to you that both implementations produce the same results?

kevinrushforth · 2019-03-11T15:57:03Z

Moreover, there are many spacing changes, such as in D3DContext.java and I'm not sure at all these are within the style conventions of OpenJFX. Kevin would have to comment on this I think.

I think these should be reverted. This sort of wholesale reformatting is discouraged because it makes changes harder to review (and can sometimes cause merge conflicts).

ProggerFox · 2019-03-13T15:02:12Z

@nlisker

Did you test with the test code I linked to you that both implementations produce the same results?

Not yet, but i plan to do just that shortly.
I admit i worked a little hastly in order to finish this issue before the next incoming flood of customer support will steal the remaining time from me.

Regarding

Moreover, there are many spacing changes, such as in D3DContext.java and I'm not sure at all these are within the style conventions of OpenJFX. Kevin would have to comment on this I think.

I took the file as it is from the sources you provided and the IDE seems to have no issues with the indents (i use spaces, no tabs) or the likes.
My own additions indeed need some reworking, albeit there are indent issues at places in the sources i didn't touched.
Surely this is not a common thing with the sources in OpenJFX? (not saying i wouldn't fix them though)

- Tabs to spaces - line at end of sources - removal of "Falco"-comments - changed policy of line separator

nlisker

Additional comments:

.idea/misc.xml has been changed again.
Undo the formatting (adding extra spaces) changes to: MeshView.java, D3DContext.java, LightBase.java and D3DMeshView.java.

nlisker · 2019-03-15T00:12:43Z

modules/javafx.graphics/src/main/native-prism-d3d/D3DLight.h


 private:

 };

 #endif  /* D3DLIGHT_H */

+


Remove empty lines.

nlisker · 2019-03-15T00:16:59Z

modules/javafx.graphics/src/main/java/com/sun/prism/es2/ES2Context.java

-        glContext.setPointLight(nativeHandle, index, x, y, z, r, g, b, w);
+    /*
+     * FacloTheBold: - new operator, added parameters for attenuation coefficients
+     */


Remove this comment if you removed the rest.

nlisker · 2019-03-15T00:25:55Z

modules/javafx.graphics/src/main/resources/com/sun/prism/es2/glsl/main1Light.frag


-    gl_FragColor = vec4(clamp(rez, 0.0, 1.0) , diffuse.a);
+        vec3 rez = (ambientColor+d) * (att * (diffuse.xyz + s*specular.rgb));


I think that att should multiply d and s (same as multiplying lights[0].color.rgb). The same is done in the d3d pipeline.

nlisker · 2019-03-15T00:30:53Z

I took the file as it is from the sources you provided and the IDE seems to have no issues with the indents (i use spaces, no tabs) or the likes.
My own additions indeed need some reworking, albeit there are indent issues at places in the sources i didn't touched.
Surely this is not a common thing with the sources in OpenJFX? (not saying i wouldn't fix them though)

The sources I uploaded do not change indentation. I'm not sure what indentation issues you are referring to.

ProggerFox · 2019-03-15T08:46:28Z

The sources I uploaded do not change indentation. I'm not sure what indentation issues you are referring to.

May i know which IDE you used for working on your sources?
I think i will switch temporarily the tools i use for editing since something is not working straight at the moment.

ProggerFox · 2019-03-15T08:50:44Z

You see a Github-rookie at its finest, sorry for the inconveniences.
I plan to squash the commits soon, but since @nlisker is now listed as reviewer, how should i go on about the squashing?

nlisker · 2019-03-15T14:22:04Z

May i know which IDE you used for working on your sources?

I use Eclipse.

nlisker · 2019-03-19T15:42:25Z

The implementation looks promising. Can you test with the app I uploaded that we get the same visuals?

nlisker · 2019-03-25T09:38:39Z

modules/javafx.graphics/src/main/resources/com/sun/prism/es2/glsl/main1Light.frag


-    vec3 rez = (ambientColor+d) * diffuse.xyz + s*specular.rgb;
-    rez += apply_selfIllum().xyz;
+        vec3 rez = (ambientColor+d) * (diffuse.xyz + s*specular.rgb);


I'm not sure that this change to the formula is correct. d should only multiply the diffuse color I think. Same for the other shaders.

nlisker · 2019-05-05T20:13:22Z

@FalcoTheBold How's it going? We are slowly running out of time to get all the planned changes into 13. We need to leave enough time for reviewers too.

ProggerFox · 2019-05-13T14:21:14Z

@FalcoTheBold How's it going? We are slowly running out of time to get all the planned changes into 13. We need to leave enough time for reviewers too.

Hello there.
At the moment i am in quite a pinch with my regular work, finding little time to work on this and my other projects. Nonetheless i am on it to test it on the Linux-systems, just lately i had a sprint with my colleague on another work, thus taking less time on the issues here. This will change in a short while however, i will update you then.

kevinrushforth · 2019-10-01T00:39:03Z

As announced in this message, the official jfx repository is moving from hg.openjdk.java.net/openjfx/jfx-dev/rt to github.com/openjdk/jfx.

This sandbox repository is being retired on 1-Oct-2019. You will need to migrate your WIP pull request to the openjdk/jfx repo if you want to continue working on it. Here are instructions for migrating your pull request. The updated CONTRIBUTING.md doc has additional information on how to proceed.

Once you have done this, it would be helpful to add a comment with a pointer to the new PR.

NOTE: since this is a feature / enhancement request it needs to be discussed on the openjfx-dev mailing list if you want it to be considered.

The new openjdk/jfx repo will be open for pull requests on Wednesday, 2-Oct-2019. I will send email to the openjfx-dev mailing list announcing this.

nlisker · 2019-12-06T22:09:00Z

PR on the Skara repo: openjdk/jfx#43

Add ES2-attenuation

c3b27b1

Adding attenuation factors to the JavaFX 3D-API

kevinrushforth added enhancement New feature or request WIP Work In Progress labels Feb 21, 2019

Small fixes javafxports#1

f60baa5

Added SetRange opeator

Small fixes javafxports#2

28ce433

- Added missing shader-files - Changed default values of attenuation-factors

ProggerFox and others added 7 commits March 6, 2019 08:10

Fixes for AppVeyor-build

d121fec

Delete LightBase.java

624a947

Delete D3DMeshView.java

f7a6e70

Fixes for AppVeyor-build

370a588

Fixes for AppVeyor-build javafxports#2

8e8eb2f

[Test] Sources for Java-side

93a0fa5

Small fixes javafxports#3

18e58fe

- added missing members for native MeshView-struct - changed 'la' to correct data-type in native setPointLight-operation

ProggerFox changed the title ~~[WIP] JDK-8217477: Add attenuation for the es2 pipeline~~ [WIP] JDK-8217477: Implement attenuation-coefficients for PointLight Mar 11, 2019

ProggerFox added 8 commits March 11, 2019 08:29

IIncluded Java+D3D-patch

3556ec7

Revert "IIncluded Java+D3D-patch"

0a5d380

This reverts commit 3556ec7.

Revert "Small fixes javafxports#3"

444cc94

This reverts commit 18e58fe.

Revert "Revert "IIncluded Java+D3D-patch""

a7d8b91

This reverts commit 0a5d380.

Revert "Revert "Small fixes javafxports#3""

5d25b78

This reverts commit 444cc94.

Update D3DPhongMaterial

21a610b

Updated D3D-files

ab0eb82

Update D3DPhongMaterial.h

4d6dd65

ProggerFox mentioned this pull request Mar 11, 2019

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

Open

nlisker suggested changes Mar 11, 2019

View reviewed changes

ES2 Attenuation-logic changes

196d0cf

Formatting changes

58be282

- Tabs to spaces - line at end of sources - removal of "Falco"-comments - changed policy of line separator

Update GLContext.c

7261cfc

nlisker suggested changes Mar 15, 2019

View reviewed changes

ProggerFox closed this Mar 15, 2019

ProggerFox reopened this Mar 15, 2019

ProggerFox added 2 commits March 18, 2019 11:48

Reworked shader-sources

a44f13f

Shader-sources: removed extra brackets

354776a

nlisker reviewed Mar 25, 2019

View reviewed changes

kevinrushforth changed the title ~~[WIP] JDK-8217477: Implement attenuation-coefficients for PointLight~~ WIP: JDK-8217477: Implement attenuation-coefficients for PointLight Oct 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: JDK-8217477: Implement attenuation-coefficients for PointLight #384

WIP: JDK-8217477: Implement attenuation-coefficients for PointLight #384

ProggerFox commented Feb 20, 2019

brcolow commented Feb 21, 2019

ProggerFox commented Feb 21, 2019

nlisker left a comment

nlisker Mar 11, 2019

ProggerFox Mar 12, 2019

nlisker Mar 11, 2019

ProggerFox Mar 12, 2019

nlisker commented Mar 11, 2019

kevinrushforth commented Mar 11, 2019

ProggerFox commented Mar 13, 2019

nlisker left a comment

nlisker Mar 15, 2019

nlisker Mar 15, 2019

nlisker Mar 15, 2019

nlisker commented Mar 15, 2019

ProggerFox commented Mar 15, 2019 •

edited

Loading

ProggerFox commented Mar 15, 2019

nlisker commented Mar 15, 2019

nlisker commented Mar 19, 2019

nlisker Mar 25, 2019

nlisker commented May 5, 2019

ProggerFox commented May 13, 2019

kevinrushforth commented Oct 1, 2019 •

edited

Loading

nlisker commented Dec 6, 2019


		gl_FragColor = vec4(clamp(rez, 0.0, 1.0) , diffuse.a);
		vec3 rez = (ambientColor+d) * (att * (diffuse.xyz + s*specular.rgb));

WIP: JDK-8217477: Implement attenuation-coefficients for PointLight #384

Are you sure you want to change the base?

WIP: JDK-8217477: Implement attenuation-coefficients for PointLight #384

Conversation

ProggerFox commented Feb 20, 2019

brcolow commented Feb 21, 2019

ProggerFox commented Feb 21, 2019

nlisker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nlisker commented Mar 11, 2019

kevinrushforth commented Mar 11, 2019

ProggerFox commented Mar 13, 2019

nlisker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nlisker commented Mar 15, 2019

ProggerFox commented Mar 15, 2019 • edited Loading

ProggerFox commented Mar 15, 2019

nlisker commented Mar 15, 2019

nlisker commented Mar 19, 2019

Choose a reason for hiding this comment

nlisker commented May 5, 2019

ProggerFox commented May 13, 2019

kevinrushforth commented Oct 1, 2019 • edited Loading

nlisker commented Dec 6, 2019

ProggerFox commented Mar 15, 2019 •

edited

Loading

kevinrushforth commented Oct 1, 2019 •

edited

Loading