JDK-8094825 : [Mac,Retina,3D] Lighting is wrong (too dark, incorrect highlight) with pixel scaling
  • Type: Bug
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 8u20
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2014-11-17
  • Updated: 2015-06-22
  • Resolved: 2015-05-23
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u60Fixed 9Fixed
Related Reports
Duplicate :  
Relates :  
Relates :  
Relates :  
Description
To reproduce, run PickTest3D. On my new Mac with a Retina display and Intel Iris graphics, the lighting looks wrong. It is both too dark, and the specular highlights are not quite in the right place. See the attached screen shot. I suspect that this is a graphics card issue.
Comments
Changeset: 06fd4252872d Author: Chien Yang <chien.yang@oracle.com> Date: 2015-05-22 23:08 -0700 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/06fd4252872d Changeset: 489dba5e1ac1 Author: Chien Yang <chien.yang@oracle.com> Date: 2015-05-22 23:21 -0700 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/489dba5e1ac1
23-05-2015

Looks good. Verified on Mac and Windows with patch for RT-27960. No more failures in unit tests. All looks good. +1
22-05-2015

BTW, to test hi-DPI on Windows (d3d) you will also need to apply the proposed fix in RT-27960.
20-05-2015

The change to d3d pipe is quite small contrarily to my earlier concern. Here is the proposed fix to es2 and d3d using the same logic pattern on both pipes: http://cr.openjdk.java.net/~ckyang/RT-39418/webrev.03/
20-05-2015

I further limit the scope of this fix to es2 pipe only. The change in d3d is solely due to interface change. The way transform information is sent to the shader is done slightly differently between es2 and d3d. We will the d3d pipe once RT-27960 is fixed. http://cr.openjdk.java.net/~ckyang/RT-39418/webrev.02/
18-05-2015

To limit the scope of change of this fix, we will apply the pixel scaling to the projViewTx at render MeshView and undo the pixel scale done in to the graphics, earlier, within the same method via worldTransform. This isn't an ideal solution but it saves us from spilling the fix into 2D rendering on a dot release. We will look into unifying the pixel scale logic for 2D and 3D rendering in 9. http://cr.openjdk.java.net/~ckyang/RT-39418/webrev.01/ The fix has been tested on Mac and no regression were introduced on Windows (d3d). Kevin has also confirmed an earlier version this fix passed all unit tests on his retina MBP.
15-05-2015

Thanks the information. Looks like the simple test programs I have aren't good enough to verify the fix. I took a closer look at the shader code and I believe I do see the problem. I will work on this tomorrow ...
08-05-2015

I don't think that multiplying by the pixel scale at that point is sufficient. Testing would seem to bear out that concern, since the results, while better, are still not quite right. 4 of the 10 failing unit tests now pass, but the other 6 still fail. Also, the specular highlights for PickTest3D are still not positioned correctly and still seems a bit darker than expected. I'll take a closer look later.
07-05-2015

Good point Jim! I will move the impl. to BG and revert the change to BSG, TG and DG in the revised webrev after Kevin's input if any.
07-05-2015

I can't speak for the correctness of where and when you perform the multiplication by the pixel scale (there are apparently other transforms going on there, but I'm not familiar with that part of the code so I'll defer to Kevin). But, I would think that get/setPixelScale() would be better to implement in BaseGraphics, not BSG since there is nothing "shader-specific" about that attribute (it may have only come up in the context of Lighting shaders, but it would/will be useful for other uses as well). Other "just remember this value" getter/setters are also implemented in BG. It would also eliminate a couple of your additional implementations as unnecessary since some of those implementations subclass from BG. I'd also make getPixelScale() final, but I notice there is inconsistency of using final in the other getters in BG... :(
07-05-2015

Please review the proposed fix. The bug is due to the data for camera and light positions to 3D shaders didn't receive the needed pixel scale if the is of retina resolution. I have made similar fix to the d3d pipe even though it isn't retina ready yet. In this process I moved the place where camera position is set to native. This is to follow the pattern of es2 pipe, and it is more correct model (we may not in 3D state). http://cr.openjdk.java.net/~ckyang/RT-39418/webrev.00/ I have tested and verified that the fix works well on both d3d and es2 pipe.
06-05-2015

Attached a simple test program to illustrate light bug on retina display. The light should casts a specular spot at the center region of the Sphere, but on a retina display this region moves to the upper left portion of the Sphere.
23-01-2015

I don't have a retina display system so I will need a reliable way to simulate it in order to fix the problem.
17-12-2014

This might be a good 8u60 candidate.
17-12-2014

So this is a general retina problem then. Thanks for confirming.
18-11-2014

It looks like the lighting code in NGShape3D trusts the values it gets from the FX layer, which has no idea about pixel scaling.
18-11-2014

I can confirm on my retina MBP with GT650M. Perhaps the light sources are not scaled by the pixel scale?
18-11-2014

Not a regression. The rendering is the same with 8u20.
17-11-2014

Here is the output from prism.verbose: Graphics Vendor: Intel Inc. Renderer: Intel Iris OpenGL Engine Version: 2.1 INTEL-8.28.32
17-11-2014