MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0029097Open CASCADE[OCCT] OCCT:Visualizationpublic2017-09-10 17:562018-08-07 09:28
Reporterkgv 
Assigned Tobugmaster 
PrioritynormalSeverityfeature 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.3.0Fixed in Version[OCCT] 7.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 Filespng file icon bug29097.png (119,076 bytes) 2018-02-13 19:10

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

-  Notes
(0073892)
git (administrator)
2018-02-08 19:49

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.
(0073893)
anv (developer)
2018-02-08 19:52

Functionality was added. Any remarks and suggestions are welcome.
(0073894)
kgv (developer)
2018-02-08 22:11

+  //! 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!
(0073904)
kgv (developer)
2018-02-09 21:21

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.
(0073947)
git (administrator)
2018-02-12 20:16

Branch CR29097 has been updated forcibly by anv.

SHA-1: f1d932352c50c9b155b7e8909050935889e6ad16
(0073948)
anv (developer)
2018-02-12 20:18

All remarks have been taken into consideration and corresponding issues were fixed. Branch was updated. Please check.
(0073949)
kgv (developer)
2018-02-12 20:36

   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.
(0073959)
kgv (developer)
2018-02-13 15:28

The test case is not representative (basing on attached screenshot) - it is difficult to distinguish different shading modes.
(0073965)
git (administrator)
2018-02-13 19:12

Branch CR29097 has been updated forcibly by anv.

SHA-1: 91efa970a26d96433e5c8c2c23cc7af25dd70e57
(0073966)
anv (developer)
2018-02-13 19:13

Fixed.
(0074031)
git (administrator)
2018-02-18 15:35

Branch CR29097 has been updated forcibly by kgv.

SHA-1: 6887affff95d95aaea8afee566995b381fecde98
(0074039)
git (administrator)
2018-02-19 12:29

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

(0074045)
git (administrator)
2018-02-19 15:42

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.

(0074046)
git (administrator)
2018-02-19 16:46

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

(0074047)
git (administrator)
2018-02-19 17:03

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.
(0074049)
git (administrator)
2018-02-19 17:29

Branch CR29097_1 has been updated forcibly by kgv.

SHA-1: 0917dc0247c55cc7118af1e683c301deffeb29ea
(0074057)
git (administrator)
2018-02-19 20:32

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

(0074058)
git (administrator)
2018-02-19 20:34

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).
(0074060)
kgv (developer)
2018-02-20 07:31

Please raise the patch.

http://jenkins-test-10.nnov.opencascade.com/view/CR29097_2-master-KGV [^]
(0074068)
bugmaster (administrator)
2018-02-20 14:05

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
(0074271)
git (administrator)
2018-03-06 15:50

Branch CR29097 has been deleted by kgv.

SHA-1: c6fc15d87202e9b7eaad79ca905ad8a34afb893c
(0074272)
git (administrator)
2018-03-06 15:50

Branch CR29097_1 has been deleted by kgv.

SHA-1: 09db5cde2d84333107033fdcee1ddadce72622b3
(0074273)
git (administrator)
2018-03-06 15:50

Branch CR29097_2 has been deleted by kgv.

SHA-1: 1a3755f511e7216e03c2d7753b045928d0cba08e

- Related Changesets
occt: master dc89236f
Timestamp: 2018-02-08 15:41:09
Author: 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).
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-01-31 08:04 kgv Relationship added related to 0029439
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 user533 Fixed in Version => 7.3.0
2018-06-29 21:19 user533 Status verified => closed
2018-08-07 09:28 kgv Relationship added related to 0030021


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker