I have prepared an 'improvement suggestion' for FMX.Types3D.TVertexBuffer, verified with 64 lines of test code in a console app. A corner case, nothing urgent, but it can make it easier to debug things.
I have prepared an 'improvement suggestion' for FMX.Types3D.TVertexBuffer, verified with 64 lines of test code in a console app. A corner case, nothing urgent, but it can make it easier to debug things.
Description:
1) TVertexBuffer is part of TMeshData which is part of TCustomMesh, which is a TShape3D, which has a TMaterialSource, which has a TMaterial, which contains the shader byte code for the VertexShader and PixelShader.
2) It is typical that user code updates Vertices, TextCoord0 and Normals of the TVertexBuffer. Normals are also updated implicitely when user code calls TMeshData.CalcFaceNormals.
P: TPoint3D;
T: TPointF;
Data.VertexBuffer.Vertices[Index] := P;
Data.VertexBuffer.TexCoord0[Index] := T;
Data.VertexBuffer.Normals[Index] := P;
4) Assume that the developer creates a new custom Material which updates Position, TexCoord and Normal as part of the VertexShader. It is no longer needed to compute TexCoords and Normals on the CPU. Only the initial position data will be filled in and uploaded to the GPU, and only when changed.
5) TVertexBuffer has a ChangeFormat method which can be used to optimize the layout of the TVertexBuffer such that space is no longer wasted for unused TVertexFormat components. TexCoord0 will be removed from the format / buffer layout.
6) It is of course wrong to assign to TexCoord0 if TexCoord0 is no longer part of the format.
7) Existing code does not change when the new vertex shader code is developed. There will still be assignments to Data.VertexBuffer.TexCoords0. And it would be convenient to allow this, for backward compatibility.
8) In this situation it would be cool if TVertexBuffer.SetTexCoord0 would check if TexCoord0 is actually part of the format and, in my opinion, exit silently with no damage done.
9) Right now the property setter will write over memory reserved for other components of the TVertexBuffer, or beyond the limits of the VertexBuffer, without raising an exception.
10) Snippets from the test project, I can paste the full text later in a comment:
vf := [TVertexFormat.Vertex];
Data.ChangeFormat(vf);
Data.VertexBuffer.Length := 2;
P := TPoint3D.Create(1, 2, 3);
T := TPointF.Create(4, 5);
Data.VertexBuffer.Vertices[0] := P;
Data.VertexBuffer.Vertices[1] := P;
Data.VertexBuffer.TexCoord0[0] := T;
P := Data.VertexBuffer.Vertices[1];
System.Writeln(P.X, P.Y, P.Z);
//should print 1 2 3, prints 4 5 3
Description:
1) TVertexBuffer is part of TMeshData which is part of TCustomMesh, which is a TShape3D, which has a TMaterialSource, which has a TMaterial, which contains the shader byte code for the VertexShader and PixelShader.
2) It is typical that user code updates Vertices, TextCoord0 and Normals of the TVertexBuffer. Normals are also updated implicitely when user code calls TMeshData.CalcFaceNormals.
P: TPoint3D;
T: TPointF;
Data.VertexBuffer.Vertices[Index] := P;
Data.VertexBuffer.TexCoord0[Index] := T;
Data.VertexBuffer.Normals[Index] := P;
4) Assume that the developer creates a new custom Material which updates Position, TexCoord and Normal as part of the VertexShader. It is no longer needed to compute TexCoords and Normals on the CPU. Only the initial position data will be filled in and uploaded to the GPU, and only when changed.
5) TVertexBuffer has a ChangeFormat method which can be used to optimize the layout of the TVertexBuffer such that space is no longer wasted for unused TVertexFormat components. TexCoord0 will be removed from the format / buffer layout.
6) It is of course wrong to assign to TexCoord0 if TexCoord0 is no longer part of the format.
7) Existing code does not change when the new vertex shader code is developed. There will still be assignments to Data.VertexBuffer.TexCoords0. And it would be convenient to allow this, for backward compatibility.
8) In this situation it would be cool if TVertexBuffer.SetTexCoord0 would check if TexCoord0 is actually part of the format and, in my opinion, exit silently with no damage done.
9) Right now the property setter will write over memory reserved for other components of the TVertexBuffer, or beyond the limits of the VertexBuffer, without raising an exception.
10) Snippets from the test project, I can paste the full text later in a comment:
vf := [TVertexFormat.Vertex];
Data.ChangeFormat(vf);
Data.VertexBuffer.Length := 2;
P := TPoint3D.Create(1, 2, 3);
T := TPointF.Create(4, 5);
Data.VertexBuffer.Vertices[0] := P;
Data.VertexBuffer.Vertices[1] := P;
Data.VertexBuffer.TexCoord0[0] := T;
P := Data.VertexBuffer.Vertices[1];
System.Writeln(P.X, P.Y, P.Z);
//should print 1 2 3, prints 4 5 3
program Project1;
ReplyDelete{$APPTYPE CONSOLE}
{$R *.res}
uses
System.SysUtils,
System.Classes,
System.Math.Vectors,
System.Types,
FMX.Types3D,
FMX.Objects3D;
type
TTestMesh = class(TCustomMesh)
public
constructor Create(AOwner: TComponent); override;
procedure Test;
end;
{ TTestMesh }
constructor TTestMesh.Create(AOwner: TComponent);
var
vf: TVertexFormats;
begin
inherited;
vf := [TVertexFormat.Vertex];
Data.ChangeFormat(vf);
Data.VertexBuffer.Length := 2;
end;
procedure TTestMesh.Test;
var
P: TPoint3D;
T: TPointF;
begin
P := TPoint3D.Create(1, 2, 3);
T := TPointF.Create(4, 5);
Data.VertexBuffer.Vertices[0] := P;
Data.VertexBuffer.Vertices[1] := P;
//if TVertexFormat.TexCoord0 in Data.VertexBuffer.Format then
Data.VertexBuffer.TexCoord0[0] := T;
P := Data.VertexBuffer.Vertices[1];
System.Writeln(P.X, P.Y, P.Z);
end;
var
m: TTestMesh;
begin
try
m := TTestMesh.Create(nil);
m.Test;
Readln;
m.Test;
except
on E: Exception do
Writeln(E.ClassName, ': ', E.Message);
end;
end.
You should post this at the quality portal so that it can be upvoted.
ReplyDeleteI still need a title for the entry. Short version of the description could be: "If Index is checked and an exception raised, Format should also be checked, but no exception raised in the property setters for TexCoord0 and siblings of TVertexBuffer."
ReplyDeleteThat might need a little polishing :D
ReplyDeleteJust seconding the request to put this in QP - great bug report, investigation, sample app. I wish all bug reports were so good :)
ReplyDeleteNice report.
ReplyDeleteThough after reading the code behind TPoint3D and friends I was convinced to never use FMX for anything 3D... If they can't get such basics right, there's no hope for the rest.
(I will edit this comment until I get it almost right, seems I will need several attemts.)
ReplyDeleteMy 3D app has only two real issues:
1) The shader source for the TLightMaterial class is missing. I want to stay compatible.
2) I replaced FMX.Context.DX11 with itself, under new name for unit and class. This is good for adding things, but I cannot distribute the complete source of my project. I need a version of the context class that I can inherit from.
https://quality.embarcadero.com/browse/RSP-15676
ReplyDeleteHi Gustav Schubert - thanks for filing that!
ReplyDeleteThose two things in your last comment sound like QPs too. If we don't distribute all source (including shader source where we normally do - I thought it was normally inline in the code though) please file a QP for the missing files. And if the DX context needs to have some tweaks to make it better usable, please file that as well, saying what you want to be able to do.
The case of the missing shader source:
ReplyDeletehttps://quality.embarcadero.com/browse/RSP-15680
I will do a QP about TContextDX11 after Update 1.