Workaround 2 FPC compilation problems, and a memory leak at LoadFromJSON

michaliskambi commented

2018-12-23 02:10:51 +00:00

(Migrated from github.com)

Hi,

First of all, thanks for developing PasGLTF ! Great job ! I decided to use it as a basis for reading glTF in Castle Game Engine. We use PasGLTF unit and convert glTF to an X3D scene graph, which is something that Castle Game Engine can process and display. If you're interested, the complete unit in Castle Game Engine that uses PasGLTF is in here: https://github.com/castle-engine/castle-engine/blob/master/src/x3d/x3dloadinternalgltf.pas . I will announce news about it soon.

I found some small issues in PasGLTF that I managed to fix :)

Compilling with FPC 3.0.4 (last stable release) with debug information results in FPC errors:

$ fpc -g PasGLTF.pas
Free Pascal Compiler version 3.0.4-r37149 [2018/11/24] for x86_64
Copyright (c) 1993-2017 by Florian Klaempfl and others
Target OS: Linux for x86-64
Compiling PasGLTF.pas
PasGLTF.pas(1953,1) Error: Asm: Duplicate label DBG_$PASGLTF_$$_TTYPE
PasGLTF.pas(1953,1) Error: Asm: Duplicate label DBGREF_$PASGLTF_$$_TTYPE
PasGLTF.pas(1953,1) Error: Asm: Duplicate label DBG_$PASGLTF_$$_TTYPE
PasGLTF.pas(1953,1) Error: Asm: Duplicate label DBGREF_$PASGLTF_$$_TTYPE
PasGLTF.pas(1953,1) Fatal: There were 4 errors compiling module, stopping

Looks like FPC doesn't handle the fact that you have

TPasGLTF.TAccessor.TType,
TPasGLTF.TSampler.TType,
TPasGLTF.TCamera.TType.

I workarounded it by renaming

TPasGLTF.TSampler.TType -> TPasGLTF.TSampler.TSamplerType,
TPasGLTF.TCamera.TType -> TPasGLTF.TCamera.TCameraType.

It looks uglier of course (using the name TSamplerType within TSampler seems unnecessary), but at least it compiles with FPC with debug information now.

WARNING: This workaround does change the public PasGLTF API. If any code is using the TSampler.TType or TCamera.TType, it will have to be adjusted now. I have adjusted the included viewer such that it compiles.

Compiling with recent FPC 3.3.1, reveals additional problem of FPC 3.3.1:

$ fpc PasGLTF.pas
Free Pascal Compiler version 3.3.1-r40366 [2018/11/24] for x86_64
Copyright (c) 1993-2018 by Florian Klaempfl and others
Target OS: Linux for x86-64
Compiling PasGLTF.pas
Compiling PasJSON.pas
Compiling PasDblStrUtils.pas
PasGLTF.pas(1901,30) Error: Incompatible type for arg no. 1: Got "$gendef124", expected "<Formal type>"
PasGLTF.pas(1918,31) Error: Incompatible type for arg no. 1: Got "$gendef124", expected "<Formal type>"
PasGLTF.pas(5772) Fatal: There were 2 errors compiling module, stopping

They are around two System.Move(@fItems[...], @fItems[...], ...) calls. I workaround them using MoveSrc, MoveDst variables.

There is a memory leak. To reproduce, you can use a minimal program like attached test_gltf_read.lpr, compile it with -gh (HeapTrc unit from FPC).

test_gltf_read.lpr.txt

$ fpc -gl -gh test_gltf_read.lpr
...
$ ./test_gltf_read ~/glTF-Sample-Models/2.0/Box/glTF/Box.gltf
Heap dump by heaptrc unit
777 memory blocks allocated : 106058/108168
509 memory blocks freed     : 98480/100368
268 unfreed memory blocks : 7578
True heap size : 557056
True free heap : 507424
Should be : 514952
Call trace for block $00007F75359F3280 size 24
  $00000000004AA8D8
  $00000000004AC740
....

The reason is that you call LoadFromJSON(TPasJSON.Parse(...)), and TPasJSON.Parse creates a new instance of TPasJSONItem. But it is never freed. LoadFromJSON doesn't free it.

Hi, First of all, thanks for developing PasGLTF ! Great job ! I decided to use it as a basis for reading glTF in _Castle Game Engine_. We use PasGLTF unit and convert glTF to an X3D scene graph, which is something that _Castle Game Engine_ can process and display. If you're interested, the complete unit in Castle Game Engine that uses PasGLTF is in here: https://github.com/castle-engine/castle-engine/blob/master/src/x3d/x3dloadinternalgltf.pas . I will announce news about it soon. I found some small issues in PasGLTF that I managed to fix :) 1. Compilling with FPC 3.0.4 (last stable release) with debug information results in FPC errors: ``` $ fpc -g PasGLTF.pas Free Pascal Compiler version 3.0.4-r37149 [2018/11/24] for x86_64 Copyright (c) 1993-2017 by Florian Klaempfl and others Target OS: Linux for x86-64 Compiling PasGLTF.pas PasGLTF.pas(1953,1) Error: Asm: Duplicate label DBG_$PASGLTF_$$_TTYPE PasGLTF.pas(1953,1) Error: Asm: Duplicate label DBGREF_$PASGLTF_$$_TTYPE PasGLTF.pas(1953,1) Error: Asm: Duplicate label DBG_$PASGLTF_$$_TTYPE PasGLTF.pas(1953,1) Error: Asm: Duplicate label DBGREF_$PASGLTF_$$_TTYPE PasGLTF.pas(1953,1) Fatal: There were 4 errors compiling module, stopping ``` Looks like FPC doesn't handle the fact that you have - `TPasGLTF.TAccessor.TType`, - `TPasGLTF.TSampler.TType`, - `TPasGLTF.TCamera.TType`. I workarounded it by renaming - `TPasGLTF.TSampler.TType` -> `TPasGLTF.TSampler.TSamplerType`, - `TPasGLTF.TCamera.TType` -> `TPasGLTF.TCamera.TCameraType`. It looks uglier of course (using the name `TSamplerType` within `TSampler` seems unnecessary), but at least it compiles with FPC with debug information now. WARNING: This workaround *does* change the public PasGLTF API. If any code is using the `TSampler.TType` or `TCamera.TType`, it will have to be adjusted now. I have adjusted the included viewer such that it compiles. 2. Compiling with recent FPC 3.3.1, reveals additional problem of FPC 3.3.1: ``` $ fpc PasGLTF.pas Free Pascal Compiler version 3.3.1-r40366 [2018/11/24] for x86_64 Copyright (c) 1993-2018 by Florian Klaempfl and others Target OS: Linux for x86-64 Compiling PasGLTF.pas Compiling PasJSON.pas Compiling PasDblStrUtils.pas PasGLTF.pas(1901,30) Error: Incompatible type for arg no. 1: Got "$gendef124", expected "<Formal type>" PasGLTF.pas(1918,31) Error: Incompatible type for arg no. 1: Got "$gendef124", expected "<Formal type>" PasGLTF.pas(5772) Fatal: There were 2 errors compiling module, stopping ``` They are around two `System.Move(@fItems[...], @fItems[...], ...)` calls. I workaround them using MoveSrc, MoveDst variables. 3. There is a memory leak. To reproduce, you can use a minimal program like attached `test_gltf_read.lpr`, compile it with -gh (HeapTrc unit from FPC). [test_gltf_read.lpr.txt](https://github.com/BeRo1985/pasgltf/files/2705697/test_gltf_read.lpr.txt) ``` $ fpc -gl -gh test_gltf_read.lpr ... $ ./test_gltf_read ~/glTF-Sample-Models/2.0/Box/glTF/Box.gltf Heap dump by heaptrc unit 777 memory blocks allocated : 106058/108168 509 memory blocks freed : 98480/100368 268 unfreed memory blocks : 7578 True heap size : 557056 True free heap : 507424 Should be : 514952 Call trace for block $00007F75359F3280 size 24 $00000000004AA8D8 $00000000004AC740 .... ``` The reason is that you call `LoadFromJSON(TPasJSON.Parse(...))`, and `TPasJSON.Parse` creates a new instance of TPasJSONItem. But it is never freed. `LoadFromJSON` doesn't free it.

BeRo1985 commented

2018-12-23 17:17:27 +00:00

(Migrated from github.com)

You should post the dbginfo issues with TType and the Move issues rather into the FPC bug tracker at https://bugs.freepascal.org/ instead to me :-) so this part => wontfix (since it are issues at FPC itself)

And the memory leak issue is fixed now. so this part => fixed

You should post the dbginfo issues with TType and the Move issues rather into the FPC bug tracker at https://bugs.freepascal.org/ instead to me :-) so this part => wontfix (since it are issues at FPC itself) And the memory leak issue is fixed now. so this part => fixed

michaliskambi commented

2018-12-24 01:42:19 +00:00

(Migrated from github.com)

Sure, I wanted to post the 2 issues as FPC bugs of course :) In the meantime, I need to have PasGLTF working, with FPC 3.0.4, so I'll carry these workarounds in my PasGLTF copy.

Pull request closed

Please reopen this pull request to perform a merge.

Rows
Columns

Workaround 2 FPC compilation problems, and a memory leak at LoadFromJSON #2

Pull request closed