[dart:ffi] sizeOf with wrong result

[Sunbreak]

Reproduce

2.12.0-259.8.beta without NullSafety

class BITMAPFILEHEADER extends ffi.Struct {
  @ffi.Uint16()
  int bfType;

  @ffi.Uint32()
  int bfSize;

  @ffi.Uint16()
  int bfReserved1;

  @ffi.Uint16()
  int bfReserved2;

  @ffi.Uint32()
  int bfOffBits;
}

2.13.0-30.0.dev with NullSafety

class BITMAPFILEHEADER extends ffi.Struct {
  @ffi.Uint16()
  external int bfType;

  @ffi.Uint32()
  external int bfSize;

  @ffi.Uint16()
  external int bfReserved1;

  @ffi.Uint16()
  external int bfReserved2;

  @ffi.Uint32()
  external int bfOffBits;
}

print(sizeOf<BITMAPFILEHEADER >()) results in 16, while it is 14

[Sunbreak]

It turns out to be struct aligment: bfType is aligned to bfSize with 4 bytes

class BITMAPFILEHEADER extends ffi.Struct {
  @ffi.Uint16()
  external int bfType;

  @ffi.Uint32()
  external int bfSize;

Related: #38158, #43257

[dcharkes]

This is expected behavior. C also rounds up the size to alignment. See this example.

We're already tracking packed structs, closing this issue.

[Sunbreak]

This is expected behavior. C also rounds up the size to alignment. See this example.

We're already tracking packed structs, closing this issue.

I have no idea why godbot.org have a 16 result. Maybe related to msvc(WINE). When running with real MSVC, it is 14 by default

BITMAPFILEHEADER is from wingdi.h: https://docs.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapfileheader

typedef struct tagBITMAPFILEHEADER {
        WORD    bfType;
        DWORD   bfSize;
        WORD    bfReserved1;
        WORD    bfReserved2;
        DWORD   bfOffBits;
} BITMAPFILEHEADER, FAR *LPBITMAPFILEHEADER, *PBITMAPFILEHEADER;

[Sunbreak]

@dcharkes @mannprerak2 This is NOT expected behavior with MSVC. Could you re-open it?

[dcharkes]

@Sunbreak windows is using packed structs by having #pragma pack(push, 1) (win32 copy showing the pragma, godbolt reproduction).

Having size 16 is expected without packing, and size 14 is expected behavior with packing.

[Sunbreak]

https://github.com/twain/twain-samples/blob/e8f0151dd73519323d99a6e94841cb1e25d1d677/TWAIN-Samples/Twain_App_sample01/src/TwainApp.cpp#L942-L950

        BITMAPFILEHEADER bmpFIH = {0};
        bmpFIH.bfType = ( (WORD) ('M' << 8) | 'B');
        bmpFIH.bfSize = nImageSize + sizeof(BITMAPFILEHEADER);
        bmpFIH.bfOffBits = sizeof(BITMAPFILEHEADER)+sizeof(BITMAPINFOHEADER)+(sizeof(RGBQUAD)*dwPaletteSize);
  
        fwrite(&bmpFIH, 1, sizeof(BITMAPFILEHEADER), pFile);
        fwrite(pDIB, 1, nImageSize, pFile);
        fclose(pFile);
        pFile = 0;

twain-samples use BITMAPFILEHEADER to write bmp files. 14 bytes works fine while 16 bytes makes the bmp file scrapped and unable to open

[dcharkes]

Yes, in the win32 API, the struct is packed. So using dart:ffi without packed structs support will produce problems.

sizeOf< BITMAPFILEHEADER >() will start producing the expected result when we land support for packed structs (#38158) and you change the struct definition to include that:

@ffi.Packed(1) // This will make sizeOf<BITMAPFILEHEADER>() be 14 instead of 16.
class BITMAPFILEHEADER extends ffi.Struct {
  @ffi.Uint16()
  external int bfType;

  @ffi.Uint32() // With packing this field is at offset 2 instead of 4 bytes.
  external int bfSize;

  @ffi.Uint16() // With packing this field is at offset 6 instead of 8 bytes.
  external int bfReserved1;

  @ffi.Uint16() // With packing this field is at offset 8 instead of 10 bytes.
  external int bfReserved2;

  @ffi.Uint32() // With packing this field is at offset 10 instead of 12 bytes.
  external int bfOffBits;
}

So, to my understanding, the problem you're facing right now is that dart:ffi does not support packed structs yet. That's why I closed this issue as a duplicate of packed-structs support. Or am I misunderstanding you, and are you facing a different issue?

[Sunbreak]

So the only workaround for now is to hack like: dart-lang/native#506 ?

// struct TW_VERSION {
//    TW_UINT16  MajorNum;
//    TW_UINT16  MinorNum;
//    TW_UINT16  Language;
//    TW_UINT16  Country;
//    TW_STR32   Info;
// };
class TWVersion extends _TWStruct<TW_VERSION> {
  static const _size = 2 + 2 + 2 + 2 + 34;

  TWVersion._fromAddress(int ptr): super._fromAddress(ptr);

  @override
  void dispose() {
    throw 'Undisposable nested structure';
  }

  @override
  int get size => _size;

  int get MajorNum => _getUint16(0);

  set MajorNum(int i) => _setUint16(0, i);

  int get MinorNum => _getUint16(2);

  set MinorNum(int i) => _setUint16(2, i);

  int get Language => _getUint16(4);

  set Language(int i) => _setUint16(4, i);

  int get Country => _getUint16(6);

  set Country(int i) => _setUint16(6, i);

  static const _InfoOffset = 2 + 2 + 2 + 2;

  String get Info => _getString(_InfoOffset, 34);

  set Info(String s) => _setString(_InfoOffset, s, 34);
}

[dcharkes]

Yes, we're working on adding support for packed structs so that you don't have to do workarounds.

P.S. package:win32 already contains a workaround for BITMAPFILEHEADER. You might be able to use / collaborate on that package.

[Sunbreak]

P.S. package:win32 already contains a workaround for BITMAPFILEHEADER. You might be able to use / collaborate on that package.

Didn't know that. Thanks a lot!

[Sunbreak]

P.S. package:win32 already contains a workaround for BITMAPFILEHEADER. You might be able to use / collaborate on that package.

It is implemented in win32: 2.x, not in win32: 1.7.4 before null-safety. What a pity