View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0029097 | Open CASCADE | OCCT:Visualization | public | 2017-09-10 17:56 | 2021-03-17 11:28 |
Reporter | kgv | Assigned To | bugmaster | ||
Priority | normal | Severity | feature | ||
Status | closed | Resolution | fixed | ||
Target Version | 7.3.0 | Fixed in Version | 7.3.0 | ||
Summary | 0029097: Visualization - allow picking Graphic3d_TypeOfShadingModel per-object | ||||
Description | Currently 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 Reproduce | N/A | ||||
Tags | No tags attached. | ||||
Test case number | v3d glsl shading_models | ||||
related to | 0028069 | closed | bugmaster | Open CASCADE | Visualization, TKOpenGl - handle flat shading model within GLSL programs |
parent of | 0029547 | closed | bugmaster | Open CASCADE | Сonfiguration, upgrade.dat - include deprecated enums into section [rename] |
parent of | 0032222 | closed | bugmaster | Open CASCADE | Visualization, V3d_Viewer - use Graphic3d_TOSM_FRAGMENT by default instead of Graphic3d_TOSM_VERTEX |
related to | 0029516 | closed | bugmaster | Open CASCADE | Visualization - eliminate redundant property Graphic3d_MaterialAspect::ReflectionMode() |
related to | 0029517 | closed | bugmaster | Open CASCADE | Visualization - introduce AlphaMode property defining alpha value handling options |
related to | 0029519 | closed | bugmaster | Open CASCADE | Visualization, TKOpenGl - fallback to Graphic3d_TOSM_FACET from Gouraud/Phong when nodal normals are undefined |
|
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. |
|
Functionality was added. Any remarks and suggestions are welcome. |
|
+ //! 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! |
|
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. |
|
Branch CR29097 has been updated forcibly by anv. SHA-1: f1d932352c50c9b155b7e8909050935889e6ad16 |
|
All remarks have been taken into consideration and corresponding issues were fixed. Branch was updated. Please check. |
|
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. |
|
The test case is not representative (basing on attached screenshot) - it is difficult to distinguish different shading modes. |
2018-02-13 19:10 developer |
bug29097.png (119,076 bytes) |
|
Branch CR29097 has been updated forcibly by anv. SHA-1: 91efa970a26d96433e5c8c2c23cc7af25dd70e57 |
|
Fixed. |
|
Branch CR29097 has been updated forcibly by kgv. SHA-1: 6887affff95d95aaea8afee566995b381fecde98 |
|
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 |
|
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. |
|
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 |
|
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. |
|
Branch CR29097_1 has been updated forcibly by kgv. SHA-1: 0917dc0247c55cc7118af1e683c301deffeb29ea |
|
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 |
|
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). |
|
Please raise the patch. http://jenkins-test-10.nnov.opencascade.com/view/CR29097_2-master-KGV |
|
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 |
|
Branch CR29097 has been deleted by kgv. SHA-1: c6fc15d87202e9b7eaad79ca905ad8a34afb893c |
|
Branch CR29097_1 has been deleted by kgv. SHA-1: 09db5cde2d84333107033fdcee1ddadce72622b3 |
|
Branch CR29097_2 has been deleted by kgv. SHA-1: 1a3755f511e7216e03c2d7753b045928d0cba08e |
occt: master dc89236f 2018-02-08 15:41:09
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 |
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 |
|
Note Added: 0073893 | |
2018-02-08 19:52 |
|
Assigned To | anv => kgv |
2018-02-08 19:52 |
|
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 |
|
Note Added: 0073948 | |
2018-02-12 20:18 |
|
Assigned To | anv => kgv |
2018-02-12 20:18 |
|
Status | assigned => resolved |
2018-02-12 20:36 | kgv | Note Added: 0073949 | |
2018-02-13 15:25 |
|
File Added: bug29097.png | |
2018-02-13 15:28 | kgv | Note Added: 0073959 | |
2018-02-13 18:40 |
|
File Deleted: bug29097.png | |
2018-02-13 18:40 |
|
File Added: bug29097.png | |
2018-02-13 19:09 |
|
File Deleted: bug29097.png | |
2018-02-13 19:10 |
|
File Added: bug29097.png | |
2018-02-13 19:12 | git | Note Added: 0073965 | |
2018-02-13 19:13 |
|
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 |
|
Fixed in Version | => 7.3.0 |
2018-06-29 21:19 |
|
Status | verified => closed |
2021-03-17 11:28 | kgv | Relationship added | parent of 0032222 |