BRNSystems/irrlicht
1
0
Fork 0
forked from Mirrorlandia_minetest/irrlicht
Code Pull Requests Activity

Fix some bitfield sizes in SMaterial

Browse Source 
Bitfields for PolygonOffsetDirection, ZWriteEnable and BlendOperation were chosen too small.
As we have pre c++11 code and therefore didn't use unsigned qualifiers for enums they were generally signed (up to compiler in theory, but I think they all choose signed).
Which means the bitfield also had a sign. 
So for example setting PolygonOffsetDirection to EPO_FRONT set it to -1 instead of 1.
Which then would fail with comparison checks (PolygonOffsetDirection == EPO_FRONT would be false).
We kind of got lucky that we usually not checked for the last enum inside Irrlicht, so it worked to due being the "else" case.
Or in the ZWriteEnable case the last one was identical to the default return value so it also worked accidentally.
But obviously still wrong and user code could be messed up.

While at it I also re-ordered SMaterial variable so most bitfield variables are close together again to give compiler at least a chance to use packing. Thought at least in my quick debug compile test it didn't seem to use any packing (but maybe on other compilers).

git-svn-id: svn://svn.code.sf.net/p/irrlicht/code/trunk@6440 dfc29bdd-3216-0410-991c-e03cc46cb475
This commit is contained in:
cutealien 2022-11-17 16:42:39 +00:00
parent 3f372af486
commit e57a713377
3 changed files with 19 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changes.txt
View File

@ -1,6 +1,7 @@
--------------------------
Changes in 1.9 (not yet released)
- Fix some bitfield sizes in SMaterial which were chosen too small for enums (PolygonOffsetDirection, ZWriteEnable, BlendOperation)
- Add ISceneNode::UpdateAbsolutePosBehavior variable to allow nodes to ignore the scale/rotation parts of their parents transformation.
- Add IGUISpinBox functions getValueFor and getOldValue
- Bugfix: IGUIElement::getNextElement now passing includeInvisible and includeDisabled flags recursively instead of disabling those for children.

34
include/SMaterial.h
View File

@ -310,8 +310,8 @@ namespace video
Shininess(0.0f), MaterialTypeParam(0.0f), MaterialTypeParam2(0.0f), Thickness(1.0f),
ZBuffer(ECFN_LESSEQUAL), AntiAliasing(EAAM_SIMPLE), ColorMask(ECP_ALL),
ColorMaterial(ECM_DIFFUSE), BlendOperation(EBO_NONE), BlendFactor(0.0f),
PolygonOffsetFactor(0), PolygonOffsetDirection(EPO_FRONT),
PolygonOffsetDepthBias(0.f), PolygonOffsetSlopeScale(0.f),
PolygonOffsetFactor(0), PolygonOffsetDirection(EPO_FRONT),
Wireframe(false), PointCloud(false), GouraudShading(true),
Lighting(true), ZWriteEnable(EZW_AUTO), BackfaceCulling(true), FrontfaceCulling(false),
FogEnable(false), NormalizeNormals(false), UseMipMaps(true)
@ -413,7 +413,7 @@ namespace video
//! Store the blend operation of choice
/** Values to be chosen from E_BLEND_OPERATION. */
E_BLEND_OPERATION BlendOperation:4;
E_BLEND_OPERATION BlendOperation:8;
//! Store the blend factors
/** textureBlendFunc/textureBlendFuncSeparate functions should be used to write
@ -427,18 +427,6 @@ namespace video
(setting it to EBO_ADD is probably the most common one value). */
f32 BlendFactor;
//! DEPRECATED. Will be removed after Irrlicht 1.9. Please use PolygonOffsetDepthBias instead.
/** Factor specifying how far the polygon offset should be made.
Specifying 0 disables the polygon offset. The direction is specified separately.
The factor can be from 0 to 7.
Note: This probably never worked on Direct3D9 (was coded for D3D8 which had different value ranges) */
u8 PolygonOffsetFactor:3;
//! DEPRECATED. Will be removed after Irrlicht 1.9.
/** Flag defining the direction the polygon offset is applied to.
Can be to front or to back, specified by values from E_POLYGON_OFFSET. */
E_POLYGON_OFFSET PolygonOffsetDirection:1;
//! A constant z-buffer offset for a polygon/line/point
/** The range of the value is driver specific.
On OpenGL you get units which are multiplied by the smallest value that is guaranteed to produce a resolvable offset.
@ -457,6 +445,18 @@ namespace video
and -1.f to pull them towards the camera. */
f32 PolygonOffsetSlopeScale;
//! DEPRECATED. Will be removed after Irrlicht 1.9. Please use PolygonOffsetDepthBias instead.
/** Factor specifying how far the polygon offset should be made.
Specifying 0 disables the polygon offset. The direction is specified separately.
The factor can be from 0 to 7.
Note: This probably never worked on Direct3D9 (was coded for D3D8 which had different value ranges) */
u8 PolygonOffsetFactor:3;
//! DEPRECATED. Will be removed after Irrlicht 1.9.
/** Flag defining the direction the polygon offset is applied to.
Can be to front or to back, specified by values from E_POLYGON_OFFSET. */
E_POLYGON_OFFSET PolygonOffsetDirection:2;
//! Draw as wireframe or filled triangles? Default: false
/** The user can access a material flag using
\code material.Wireframe=true \endcode
@ -475,7 +475,7 @@ namespace video
//! Is the zbuffer writable or is it read-only. Default: EZW_AUTO.
/** If this parameter is not EZW_OFF, you probably also want to set ZBuffer
to values other than ECFN_DISABLED */
E_ZWRITE ZWriteEnable:2;
E_ZWRITE ZWriteEnable:3;
//! Is backface culling enabled? Default: true
bool BackfaceCulling:1;
@ -710,10 +710,10 @@ namespace video
ColorMaterial != b.ColorMaterial ||
BlendOperation != b.BlendOperation ||
BlendFactor != b.BlendFactor ||
PolygonOffsetFactor != b.PolygonOffsetFactor ||
PolygonOffsetDirection != b.PolygonOffsetDirection ||
PolygonOffsetDepthBias != b.PolygonOffsetDepthBias ||
PolygonOffsetSlopeScale != b.PolygonOffsetSlopeScale ||
PolygonOffsetFactor != b.PolygonOffsetFactor ||
PolygonOffsetDirection != b.PolygonOffsetDirection ||
UseMipMaps != b.UseMipMaps
;
for (u32 i=0; (i<MATERIAL_MAX_TEXTURES_USED) && !different; ++i)

2
tests/tests-last-passed-at.txt
View File

@ -1,4 +1,4 @@
Tests finished. 72 tests of 72 passed.
Compiled as DEBUG
Test suite pass at GMT Sat Oct 15 15:38:53 2022
Test suite pass at GMT Thu Nov 17 16:31:49 2022