View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0029097Open CASCADEOCCT:Visualizationpublic2017-09-10 17:562021-03-17 11:28
Reporterkgv Assigned Tobugmaster  
PrioritynormalSeverityfeature 
Status closedResolutionfixed 
Target Version7.3.0Fixed in Version7.3.0 
Summary0029097: Visualization - allow picking Graphic3d_TypeOfShadingModel per-object
DescriptionCurrently Graphic3d_TypeOfShadingModel can be set for entire View.
This limits usage of this functionality, because usually it is required to customize shading model on per-object level (e.g. it is weird applying Flat shading model on trihedron at the corner of the View).
Steps To ReproduceN/A
TagsNo tags attached.
Test case numberv3d glsl shading_models

Attached Files

  • bug29097.png (119,076 bytes)

Relationships

related to 0028069 closedbugmaster Open CASCADE Visualization, TKOpenGl - handle flat shading model within GLSL programs 
parent of 0029547 closedbugmaster Open CASCADE Сonfiguration, upgrade.dat - include deprecated enums into section [rename] 
parent of 0032222 closedbugmaster Open CASCADE Visualization, V3d_Viewer - use Graphic3d_TOSM_FRAGMENT by default instead of Graphic3d_TOSM_VERTEX 
related to 0029516 closedbugmaster Open CASCADE Visualization - eliminate redundant property Graphic3d_MaterialAspect::ReflectionMode() 
related to 0029517 closedbugmaster Open CASCADE Visualization - introduce AlphaMode property defining alpha value handling options 
related to 0029519 closedbugmaster Open CASCADE Visualization, TKOpenGl - fallback to Graphic3d_TOSM_FACET from Gouraud/Phong when nodal normals are undefined 

Activities

git

2018-02-08 19:49

administrator   ~0073892

Branch CR29097 has been created by anv.

SHA-1: cedaa406e267bd8a7fd7c8e52184bb893eb1f3d7


Detailed log of new commits:

Author: anv
Date: Thu Feb 8 18:41:09 2018 +0300

    0029097: Visualization - allow picking shading model per-object
    
    - Added possibility to set a personal Shading Model for an AspectFillArea3d and build shading program basing on it;
    - Added new element for Graphic3d_TypeOfShadingModel enumeration to define objects that has no own Shading Model;
    - V3d_TypeOfShadingModel is unnecessary and replaced as typedef to Graphic3d_TypeOfShadingModel for backward compatibility;
    - Added new Draw command vshadingmodel to test this feature.

anv

2018-02-08 19:52

developer   ~0073893

Functionality was added. Any remarks and suggestions are welcome.

kgv

2018-02-08 22:11

developer   ~0073894 

+  //! Returns shading model
+  Graphic3d_TypeOfShadingModel ShadingModel() const { return myShadingModel; }

Please document the default value (see other getters in class).

 //! Graphic3d_TOSM_FRAGMENT Interpolation of color based on normals (Phong   Shading)
 enum Graphic3d_TypeOfShadingModel
 {
+  Graphic3d_TOSM_DEFAULT,
   Graphic3d_TOSM_NONE,
   Graphic3d_TOSM_FACET,
   Graphic3d_TOSM_VERTEX,

Please add documentation for each enumeration value using //!< syntax.

+  Standard_EXPORT bool SetShadingModel (const Graphic3d_TypeOfShadingModel theModel,
+                                        const bool                         theToOverrideDefaults = false);

Documentation is missing.

-enum V3d_TypeOfShadingModel
-{
-V3d_COLOR,
-V3d_FLAT,
-V3d_GOURAUD,
-V3d_PHONG
-};

You have just removed enumeration - please provide compatibility fallback as in case of V3d_TypeOfLight/Graphic3d_TypeOfLightSource.

-                              const V3d_TypeOfShadingModel theShadingModel = V3d_GOURAUD,
+                              const V3d_TypeOfShadingModel theShadingModel = Graphic3d_TOSM_VERTEX,

Please replace existing occurrences of V3d_TypeOfShadingModel from OCCT and samples (the definition should be preserved only for porting).

+static Standard_Integer VShadingModel (Draw_Interpretor& /*theDI*/,
+                                     Standard_Integer  theArgNb,
+                                     const char**      theArgVec)

Please don't add new command - extend functionality of existing commands (like vaspects).

+    if (anArg == "color")
+      aModel = Graphic3d_TOSM_NONE;
+    else if (anArg == "flat")
+      aModel = Graphic3d_TOSM_FACET;
+    else if (anArg == "gouraud")
+      aModel = Graphic3d_TOSM_VERTEX;
+    else if (anArg == "phong")
+      aModel = Graphic3d_TOSM_FRAGMENT;

Please handle aliases as in case of VRenderParams command (or better - try sharing the parsing code).

+Handle(OpenGl_ShaderProgram) OpenGl_ShaderManager::OwnShaderProgram 
...
+  TCollection_AsciiString aKey;
+  if (!aLights.IsNull())
+  {
+    aKey = THE_SHADINGMODEL_KEY_LETTERS[theProgramType];
+    aKey += "_";

The point of this patch is providing an efficient interface for switching Shading Model program.
Current patch implies significant overhead for querying non-default standard program, which can be acceptable in limited use cases, but unacceptable in general - the Shading Model management code should be revised within OpenGl_ShaderManager to be moved into bits or into nested array.

Test case is missing!

kgv

2018-02-09 21:21

developer   ~0073904

There should be also a test case for scenario when Triangulation Array is defined without normals and shading model is set to Graphic3d_TOSM_FACET.

vdrawparray can be used for defining such array.

Currently, such an array can be drawn only without shading at all; with the patch and Graphic3d_TOSM_FACET set to the object - it should be drawn shaded.

git

2018-02-12 20:16

administrator   ~0073947

Branch CR29097 has been updated forcibly by anv.

SHA-1: f1d932352c50c9b155b7e8909050935889e6ad16

anv

2018-02-12 20:18

developer   ~0073948

All remarks have been taken into consideration and corresponding issues were fixed. Branch was updated. Please check.

kgv

2018-02-12 20:36

developer   ~0073949 

   Graphic3d_MaterialAspect        myBackMaterial;
+  Graphic3d_TypeOfShadingModel    myShadingModel

Please move in front of myInteriorStyle.

+//! Compute own Shader Program for the object if, one is set
+Handle(OpenGl_ShaderProgram) OpenGl_ShaderManager::OwnShaderProgram

This does not match OCCT Coding Style.

v3d/glsl/shading_per_object

Please attach screenshot of the new test to this bug.

kgv

2018-02-13 15:28

developer   ~0073959

The test case is not representative (basing on attached screenshot) - it is difficult to distinguish different shading modes.

anv

2018-02-13 19:10

developer  

bug29097.png (119,076 bytes)

git

2018-02-13 19:12

administrator   ~0073965

Branch CR29097 has been updated forcibly by anv.

SHA-1: 91efa970a26d96433e5c8c2c23cc7af25dd70e57

anv

2018-02-13 19:13

developer   ~0073966

Fixed.

git

2018-02-18 15:35

administrator   ~0074031

Branch CR29097 has been updated forcibly by kgv.

SHA-1: 6887affff95d95aaea8afee566995b381fecde98

git

2018-02-19 12:29

administrator   ~0074039

Branch CR29097 has been updated by kgv.

SHA-1: 87d4aaf2f280e1c9d7d941e795a6a77c36dd5c24


Detailed log of new commits:

Author: kgv
Date: Mon Feb 19 12:28:57 2018 +0300

    cosmetics

git

2018-02-19 15:42

administrator   ~0074045

Branch CR29097 has been updated by kgv.

SHA-1: 3f19c17a2880e3b967323a0b5ecada69dffa18bb


Detailed log of new commits:

Author: kgv
Date: Mon Feb 19 15:42:46 2018 +0300

    OpenGl_SetOfShaderPrograms now holds an array of Shading Models.

git

2018-02-19 16:46

administrator   ~0074046

Branch CR29097 has been updated by kgv.

SHA-1: c6fc15d87202e9b7eaad79ca905ad8a34afb893c


Detailed log of new commits:

Author: kgv
Date: Mon Feb 19 16:45:08 2018 +0300

    # adjusted test case

git

2018-02-19 17:03

administrator   ~0074047

Branch CR29097_1 has been created by kgv.

SHA-1: 5fec12c3c8783f7f6950be67f7fbf1f830476bcf


Detailed log of new commits:

Author: anv
Date: Thu Feb 8 18:41:09 2018 +0300

    0029097: Visualization - allow picking Graphic3d_TypeOfShadingModel per-object
    
    Graphic3d_AspectFillArea3d has been extended by new property ::ShadingModel(),
    which is set to Graphic3d_TOSM_DEFAULT by default.
    The new API allows assigning Shading Model to specific Primitive Array groups
    instead of entire Viewer, which was the only possibility before.
    
    Graphic3d_TypeOfShadingModel has been extended with Graphic3d_TOSM_DEFAULT value
    meaining that Shading Model defined as default for the Viewer should be used.
    Graphic3d_TOSM_NONE has been renamed to Graphic3d_TOSM_UNLIT.
    Documentation of Shading Models has been improved by more details.
    
    V3d_TypeOfShadingModel enumeration has been merged into Graphic3d_TypeOfShadingModel
    avoiding duplicated definitions and confusion.
    Old values has been left for compatibility with old code and can be marked deprecated in future.
    
    Draw Harness command vaspects has been extended by new argument -setShadingModel
    for testing Shading Models assigned to entire objects.
    
    OpenGl_SetOfShaderPrograms now holds an array of Shading Models.
    OpenGl_ShaderManager interface has been modified and now requires enumeration as input
    in several places where Boolean flags have been used previously
    (methods ::BindFaceProgram(), ::BindLineProgram(), ::BindMarkerProgram()).
    
    OpenGl_Workspace now defines default (undefined) OpenGl_AspectFace as Graphic3d_TOSM_UNLIT
    to simplify indication of primitive groups with undefined Fill Area aspects,
    and so that Graphic3d_TOSM_UNLIT set as default Shading Model will not make artifacts on Lines and Markers.

git

2018-02-19 17:29

administrator   ~0074049

Branch CR29097_1 has been updated forcibly by kgv.

SHA-1: 0917dc0247c55cc7118af1e683c301deffeb29ea

git

2018-02-19 20:32

administrator   ~0074057

Branch CR29097_1 has been updated by kgv.

SHA-1: 09db5cde2d84333107033fdcee1ddadce72622b3


Detailed log of new commits:

Author: kgv
Date: Mon Feb 19 20:32:53 2018 +0300

    AIS_Manipulator::Axis::Compute() - add missing initialization of Fill Area aspects

git

2018-02-19 20:34

administrator   ~0074058

Branch CR29097_2 has been created by kgv.

SHA-1: 1a3755f511e7216e03c2d7753b045928d0cba08e


Detailed log of new commits:

Author: anv
Date: Thu Feb 8 18:41:09 2018 +0300

    0029097: Visualization - allow picking Graphic3d_TypeOfShadingModel per-object
    
    Graphic3d_AspectFillArea3d has been extended by new property ::ShadingModel(),
    which is set to Graphic3d_TOSM_DEFAULT by default.
    The new API allows assigning Shading Model to specific Primitive Array groups
    instead of entire Viewer, which was the only possibility before.
    
    Graphic3d_TypeOfShadingModel has been extended with Graphic3d_TOSM_DEFAULT value
    meaining that Shading Model defined as default for the Viewer should be used.
    Graphic3d_TOSM_NONE has been renamed to Graphic3d_TOSM_UNLIT.
    Documentation of Shading Models has been improved by more details.
    
    V3d_TypeOfShadingModel enumeration has been merged into Graphic3d_TypeOfShadingModel
    avoiding duplicated definitions and confusion.
    Old values has been left for compatibility with old code and can be marked deprecated in future.
    
    Draw Harness command vaspects has been extended by new argument -setShadingModel
    for testing Shading Models assigned to entire objects.
    
    OpenGl_SetOfShaderPrograms now holds an array of Shading Models.
    OpenGl_ShaderManager interface has been modified and now requires enumeration as input
    in several places where Boolean flags have been used previously
    (methods ::BindFaceProgram(), ::BindLineProgram(), ::BindMarkerProgram()).
    
    OpenGl_Workspace now defines default (undefined) OpenGl_AspectFace as Graphic3d_TOSM_UNLIT
    to simplify indication of primitive groups with undefined Fill Area aspects,
    and so that Graphic3d_TOSM_UNLIT set as default Shading Model will not make artifacts on Lines and Markers.
    
    AIS_Manipulator::Axis::Compute() - added missing initialization of Fill Area aspects (leading to undefined behavior).

kgv

2018-02-20 07:31

developer   ~0074060

Please raise the patch.

http://jenkins-test-10.nnov.opencascade.com/view/CR29097_2-master-KGV

bugmaster

2018-02-20 14:05

administrator   ~0074068

Combination -
OCCT branch : CR29097_2 SHA - 1a3755f511e7216e03c2d7753b045928d0cba08e
Products branch : master SHA - d452be1d47046de2f00debc4c4fe2166f6a9904d
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian70-64:
OCCT
Total CPU difference: 18214.33999999965 / 18347.729999999596 [-0.73%]
Products
Total CPU difference: 7491.350000000001 / 7477.510000000006 [+0.19%]
Windows-64-VC10:
OCCT
Total CPU difference: 17653.71276409856 / 17722.010001898587 [-0.39%]
Products
Total CPU difference: 8002.086895099984 / 8043.3491595999785 [-0.51%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2018-03-06 15:50

administrator   ~0074271

Branch CR29097 has been deleted by kgv.

SHA-1: c6fc15d87202e9b7eaad79ca905ad8a34afb893c

git

2018-03-06 15:50

administrator   ~0074272

Branch CR29097_1 has been deleted by kgv.

SHA-1: 09db5cde2d84333107033fdcee1ddadce72622b3

git

2018-03-06 15:50

administrator   ~0074273

Branch CR29097_2 has been deleted by kgv.

SHA-1: 1a3755f511e7216e03c2d7753b045928d0cba08e

Related Changesets

occt: master dc89236f

2018-02-08 15:41:09

anv


Committer: bugmaster Details Diff		 0029097: Visualization - allow picking Graphic3d_TypeOfShadingModel per-object

Graphic3d_AspectFillArea3d has been extended by new property ::ShadingModel(),
which is set to Graphic3d_TOSM_DEFAULT by default.
The new API allows assigning Shading Model to specific Primitive Array groups
instead of entire Viewer, which was the only possibility before.

Graphic3d_TypeOfShadingModel has been extended with Graphic3d_TOSM_DEFAULT value
meaining that Shading Model defined as default for the Viewer should be used.
Graphic3d_TOSM_NONE has been renamed to Graphic3d_TOSM_UNLIT.
Documentation of Shading Models has been improved by more details.

V3d_TypeOfShadingModel enumeration has been merged into Graphic3d_TypeOfShadingModel
avoiding duplicated definitions and confusion.
Old values has been left for compatibility with old code and can be marked deprecated in future.

Draw Harness command vaspects has been extended by new argument -setShadingModel
for testing Shading Models assigned to entire objects.

OpenGl_SetOfShaderPrograms now holds an array of Shading Models.
OpenGl_ShaderManager interface has been modified and now requires enumeration as input
in several places where Boolean flags have been used previously
(methods ::BindFaceProgram(), ::BindLineProgram(), ::BindMarkerProgram()).

OpenGl_Workspace now defines default (undefined) OpenGl_AspectFace as Graphic3d_TOSM_UNLIT
to simplify indication of primitive groups with undefined Fill Area aspects,
and so that Graphic3d_TOSM_UNLIT set as default Shading Model will not make artifacts on Lines and Markers.

AIS_Manipulator::Axis::Compute() - added missing initialization of Fill Area aspects (leading to undefined behavior).		 Affected Issues
0029097
mod - dox/dev_guides/upgrade/upgrade.md Diff File
mod - src/AIS/AIS_Manipulator.cxx Diff File
mod - src/Graphic3d/Graphic3d_AspectFillArea3d.cxx Diff File
mod - src/Graphic3d/Graphic3d_AspectFillArea3d.hxx Diff File
mod - src/Graphic3d/Graphic3d_CView.cxx Diff File
mod - src/Graphic3d/Graphic3d_CView.hxx Diff File
mod - src/Graphic3d/Graphic3d_TypeOfShadingModel.hxx Diff File
mod - src/OpenGl/OpenGl_AspectFace.cxx Diff File
mod - src/OpenGl/OpenGl_AspectFace.hxx Diff File
mod - src/OpenGl/OpenGl_Context.cxx Diff File
mod - src/OpenGl/OpenGl_PrimitiveArray.cxx Diff File
mod - src/OpenGl/OpenGl_SetOfShaderPrograms.hxx Diff File
mod - src/OpenGl/OpenGl_ShaderManager.cxx Diff File
mod - src/OpenGl/OpenGl_ShaderManager.hxx Diff File
mod - src/OpenGl/OpenGl_Text.cxx Diff File
mod - src/OpenGl/OpenGl_TextureSet.cxx Diff File
mod - src/OpenGl/OpenGl_TextureSet.hxx Diff File
mod - src/OpenGl/OpenGl_View.cxx Diff File
mod - src/OpenGl/OpenGl_View.hxx Diff File
mod - src/OpenGl/OpenGl_View_Raytrace.cxx Diff File
mod - src/OpenGl/OpenGl_View_Redraw.cxx Diff File
mod - src/OpenGl/OpenGl_Workspace.cxx Diff File
mod - src/Prs3d/Prs3d_Drawer.cxx Diff File
mod - src/Prs3d/Prs3d_Drawer.hxx Diff File
mod - src/V3d/V3d_TypeOfShadingModel.hxx Diff File
mod - src/V3d/V3d_View.cxx Diff File
mod - src/V3d/V3d_View.hxx Diff File
mod - src/V3d/V3d_Viewer.cxx Diff File
mod - src/V3d/V3d_Viewer.hxx Diff File
mod - src/ViewerTest/ViewerTest.cxx Diff File
mod - src/ViewerTest/ViewerTest.hxx Diff File
mod - src/ViewerTest/ViewerTest_OpenGlCommands.cxx Diff File
mod - src/ViewerTest/ViewerTest_ViewerCommands.cxx Diff File
add - tests/v3d/glsl/shading_models Diff File

Issue History

Date Modified Username Field Change
2017-09-10 17:56 kgv New Issue
2017-09-10 17:56 kgv Assigned To => kgv
2017-09-10 17:56 kgv Relationship added related to 0028069
2018-01-31 08:04 kgv Assigned To kgv => anv
2018-01-31 08:04 kgv Status new => assigned
2018-02-08 19:49 git Note Added: 0073892
2018-02-08 19:52 anv Note Added: 0073893
2018-02-08 19:52 anv Assigned To anv => kgv
2018-02-08 19:52 anv Status assigned => resolved
2018-02-08 22:11 kgv Note Added: 0073894
2018-02-08 22:11 kgv Assigned To kgv => anv
2018-02-08 22:11 kgv Status resolved => assigned
2018-02-09 21:21 kgv Note Added: 0073904
2018-02-12 20:16 git Note Added: 0073947
2018-02-12 20:18 anv Note Added: 0073948
2018-02-12 20:18 anv Assigned To anv => kgv
2018-02-12 20:18 anv Status assigned => resolved
2018-02-12 20:36 kgv Note Added: 0073949
2018-02-13 15:25 anv File Added: bug29097.png
2018-02-13 15:28 kgv Note Added: 0073959
2018-02-13 18:40 anv File Deleted: bug29097.png
2018-02-13 18:40 anv File Added: bug29097.png
2018-02-13 19:09 anv File Deleted: bug29097.png
2018-02-13 19:10 anv File Added: bug29097.png
2018-02-13 19:12 git Note Added: 0073965
2018-02-13 19:13 anv Note Added: 0073966
2018-02-18 15:35 git Note Added: 0074031
2018-02-19 12:29 git Note Added: 0074039
2018-02-19 15:42 git Note Added: 0074045
2018-02-19 16:46 git Note Added: 0074046
2018-02-19 16:47 kgv Summary Visualization - allow picking shading model per-object => Visualization - allow picking Graphic3d_TypeOfShadingModel per-object
2018-02-19 17:03 git Note Added: 0074047
2018-02-19 17:05 kgv Target Version 7.4.0 => 7.3.0
2018-02-19 17:29 git Note Added: 0074049
2018-02-19 20:32 git Note Added: 0074057
2018-02-19 20:34 git Note Added: 0074058
2018-02-19 22:24 kgv Relationship added related to 0029516
2018-02-19 22:31 kgv Relationship added related to 0029517
2018-02-20 07:31 kgv Note Added: 0074060
2018-02-20 07:31 kgv Assigned To kgv => bugmaster
2018-02-20 07:32 kgv Status resolved => reviewed
2018-02-20 07:44 kgv Relationship added related to 0029519
2018-02-20 14:05 bugmaster Note Added: 0074068
2018-02-20 14:05 bugmaster Status reviewed => tested
2018-02-20 14:06 bugmaster Test case number => v3d glsl shading_models
2018-03-04 15:34 bugmaster Changeset attached => occt master dc89236f
2018-03-04 15:34 bugmaster Status tested => verified
2018-03-04 15:34 bugmaster Resolution open => fixed
2018-03-06 12:29 kgv Relationship added parent of 0029547
2018-03-06 15:50 git Note Added: 0074271
2018-03-06 15:50 git Note Added: 0074272
2018-03-06 15:50 git Note Added: 0074273
2018-06-29 21:15 aiv Fixed in Version => 7.3.0
2018-06-29 21:19 aiv Status verified => closed
2021-03-17 11:28 kgv Relationship added parent of 0032222