[Pkg-pascal-devel] Bug#891685: castle-game-engine: FTBFS on m68k: color and vector tests fail

[Michalis Kamburelis]

Hello,

>From my side (upstream):

1. I fixed the compilation with ENDIAN_BIG in this commit:
https://github.com/castle-engine/castle-engine/commit/63ccfc4dd327fc4de1c71f8b351e1ab1933ba47d
. It's now pushed to CGE master.

    I just removed the swap -- just like Abou said, indeed the swap in
this case wasn't sensible at all.

    Note that I tested ENDIAN_BIG compilation now in a rather stupid
way (I just define it manually, while actually running compiler on
regular x86_64). I didn't really run it on big-endian machine.

2. Abou's idea with "case" in record -- that's just an excellent idea.
Thank you, I have implemented it now :)

    Details:

    - Note that it would not solve the faulty code in
src/scene/load/x3dloadinternal3ds.pas , doing stuff like "Col3Byte[2]
:= b;" , because "AsBytes" is not a default property.

    - Note that all record properties have to stay read-only otherwise
we run into a risk of "traps". This is described on
https://castle-engine.io/coding_traps and at comment "Note: We avoid
introducing in these records *any* method that changes the current
record value." in
https://github.com/castle-engine/castle-engine/blob/master/src/base/castlevectors_generic_float_record.inc
. So the writes to records must happen directly through fields. Using
"case" allows us to expose fields in more ways -- great!

    - Using "case" is an excellent idea to have writeable Data, and
also "subsets" like XY inside TVector3, XYZ inside TVector4.

    It's now done on branch
https://github.com/castle-engine/castle-engine/tree/vectors-data-as-case
. I'll merge it tomorrow if it will pass all automatic Jenkins tests
(with various FPC and Delphi versions -- but I did test that FPC
3.2.0, 3.2.2, trunk and Delphi 11 all allow "case" in advanced
records).

Thanks!
Michalis

niedz., 9 kwi 2023 o 18:09 Abou Al Montacir <abou.almontacir at sfr.fr> napisał(a):
>
> Hi,
>
> On Thu, 2022-05-26 at 20:23 +0200, Abou Al Montacir wrote:
>
> CGE build correctly on m68k architecture now.
>
> This was broken again in new upstream 7.0~alpha.2 release.
>
> There was a rework of TVector3Byte where the base data was changed from an array to a record fields and a default property was added to allow array like access.
> However, this array like property is read only, while a load from stream function tries, in big endian mode, to switch bytes.
> While I don't think this swap endianess code is needed at all, I would preferred a faster implementation with basic records:
>
>   TVector3Byte = packed record
>
>   case Boolean of:
>
>     false: (x, y, z: Byte);
>
>     true: (AsBytes: array[0..2] of Byte);
>
>   end;
>
>
> Anyway, I'll let upstream fix this as they wants, but probably this will never be in Bookworms.
>
> --
>
> Cheers,
> Abou Al Montacir