View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0024402Open CASCADEOCCT:Visualizationpublic2013-11-26 14:192014-11-11 12:51
ReporterduvAssigned Toapn  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version6.7.0 
Target Version6.8.0Fixed in Version6.8.0 
Summary0024402: TKOpenGl - Implement clipping planes in Phong GLSL program
DescriptionClipping planes not implemented yet.

It might be better to implement clip planes before further Phong program optimization for old hardware.
TagsNo tags attached.
Test case numberNot needed

Relationships

related to 0024323 closedapn TKOpenGl - spot light sources are not handled in Phong GLSL program 

Activities

duv

2013-11-26 18:48

developer   ~0026934

Dear kgv,

Please review implementation of clip planes.

kgv

2013-11-26 19:46

developer   ~0026935

Last edited: 2013-11-26 19:48

 
+  PositionWorld = occModelWorldMatrix * occVertex; // position in the view space
+  Position = occWorldViewMatrix * PositionWorld;   // position in the view space
+  Normal   = TransformNormal (occNormal);          // normal   in the view space

comments are inconsistent.
Please drop them since varying variables are already documented on declaration.

+    vec4 clipEquation = occClipPlaneEquations[anIndex];
+    int clipSpace = occClipPlaneSpaces[anIndex];

Please follow name conventions.

+    if (clipSpace == OccEquationCoords_World)
+      if (dot (clipEquation.xyz, PositionWorld) + clipEquation.w < 0.0) discard;
+      
+    if (clipSpace == OccEquationCoords_View)

clipSpace comparison conditions are exclusive. Please use "else if" to make intentions more clear.

   for (int anIndex = 0; anIndex < THE_MAX_LIGHTS; ++anIndex)
   {
+    if (anIndex >= occLightSourcesCount) break;

construction should be checked for compatibility with old hardware.

Clip planes implemented in Phong programm.

Commit description doesn't meet name conventions.

duv

2013-11-29 17:48

developer   ~0027010

Dear kgv,

Remarks considered.

kgv

2013-12-02 09:23

developer   ~0027019

Last edited: 2013-12-02 09:27

 
-  GLint*       aSpaces     = new GLint      [aPlanesNb];
+  const Standard_Size MAX_CLIP_PLANES = 8;
+
+  OpenGl_Vec4* anEquations = new OpenGl_Vec4[MAX_CLIP_PLANES];

There are no overflow checks in the following loop.
Please also consider adding test case with extremely large clipping planes number to check that we have required protections.
Also it would make sense to replace ad-hoc dynamic allocations within NCollection_Array1.

kgv

2014-02-04 21:22

developer   ~0027755 

   for (int anIndex = 0; anIndex < THE_MAX_LIGHTS; ++anIndex)
   {
+    if (anIndex >= occLightSourcesCount) break;
+

Unfortunately, this trick doesn't work for G70 (tested on GeForce 7800) - shader compilation has failed. It seems code generation is the only choice to provide compatibility within acceptable performance on old hardware or require OpenGL 3.0+ hardware.

kgv

2014-02-12 15:02

developer   ~0027858

Last edited: 2014-02-12 15:02

 Updated patch is ready for testing in CR24402_2 branch (note that it is based on 0024323). Fixed GLSL compilation issue on Intel drivers.

Note that patch introduce noticeable performance drop for fragment-intensive computations. But it is considered acceptable for common scenarios and might be improved in future if needed.

mkv

2014-02-13 11:56

tester   ~0027879

Dear BugMaster,

Branch CR24402_2 (and products from GIT master) was compiled on Linux and Windows platforms and tested.
SHA-1: ba0abec63ffaa0fe3b6dc860b2e1e1d75cf13b15

Number of compiler warnings:

occt component :
Linux: 40 (40 on master)
Windows: 0 (0 on master)

products component :
Linux: 12 (12 on master)
Windows: 2 (2 on master)

Regressions/Differences:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 380497700 / 381723732
Total CPU difference: 56795.14999999969 / 42213.76

Testing on Windows:
Total MEMORY difference: 428652520 / 431123492
Total CPU difference: 33466.609375 / 34130.65625

There are not differences in images found by testdiff.

Related Changesets

occt: master 5495fa7e

2014-02-13 09:06:51

duv


Committer: apn Details Diff		 0024402: TKOpenGl - Implement clipping planes in Phong GLSL program

Limit number of lights (breaks compatibility with old hardware).		 Affected Issues
0024402
mod - src/OpenGl/OpenGl_ShaderManager.cxx Diff File
mod - src/OpenGl/OpenGl_ShaderProgram.cxx Diff File
mod - src/OpenGl/OpenGl_ShaderProgram.hxx Diff File
mod - src/Shaders/Declarations.glsl Diff File
mod - src/Shaders/PhongShading.fs Diff File
mod - src/Shaders/PhongShading.vs Diff File

Issue History

Date Modified Username Field Change
2013-11-26 14:19 duv New Issue
2013-11-26 14:19 duv Assigned To => duv
2013-11-26 14:20 duv Relationship added related to 0024323
2013-11-26 18:48 duv Note Added: 0026934
2013-11-26 18:48 duv Assigned To duv => kgv
2013-11-26 18:48 duv Status new => resolved
2013-11-26 19:46 kgv Note Added: 0026935
2013-11-26 19:46 kgv Assigned To kgv => duv
2013-11-26 19:46 kgv Status resolved => assigned
2013-11-26 19:48 kgv Note Edited: 0026935
2013-11-29 17:48 duv Note Added: 0027010
2013-11-29 17:48 duv Assigned To duv => kgv
2013-11-29 17:48 duv Status assigned => resolved
2013-12-02 09:23 kgv Note Added: 0027019
2013-12-02 09:23 kgv Assigned To kgv => duv
2013-12-02 09:23 kgv Status resolved => assigned
2013-12-02 09:26 kgv Note Edited: 0027019
2013-12-02 09:27 kgv Note Edited: 0027019
2014-02-04 21:22 kgv Note Added: 0027755
2014-02-12 14:59 kgv Assigned To duv => kgv
2014-02-12 14:59 kgv Status assigned => resolved
2014-02-12 15:02 kgv Note Added: 0027858
2014-02-12 15:02 kgv Assigned To kgv => bugmaster
2014-02-12 15:02 kgv Status resolved => reviewed
2014-02-12 15:02 kgv Note Edited: 0027858
2014-02-12 15:53 mkv Assigned To bugmaster => mkv
2014-02-13 11:56 mkv Note Added: 0027879
2014-02-13 11:57 mkv Test case number => Not needed
2014-02-13 11:57 mkv Assigned To mkv => bugmaster
2014-02-13 11:57 mkv Status reviewed => tested
2014-02-14 12:00 apn Changeset attached => occt master 5495fa7e
2014-02-14 12:00 apn Assigned To bugmaster => apn
2014-02-14 12:00 apn Status tested => verified
2014-02-14 12:00 apn Resolution open => fixed
2014-04-04 12:36 abv Target Version 6.7.1 => 6.8.0
2014-11-11 12:47 aiv Fixed in Version => 6.8.0
2014-11-11 12:51 aiv Status verified => closed