richardwilkes / unison

A unified graphical user experience toolkit for Go desktop applications
Mozilla Public License 2.0
202 stars 10 forks source link

Skia code fails to build on 32-bit platforms #46

Closed JamesDunne closed 10 months ago

JamesDunne commented 10 months ago

I'm using xgo to build a cross-platform unison app for Linux, Windows, and MacOS but the Linux builds are failing to build on 32-bit architectures (I think; it's hard to tell which platform is being built for in the xgo output).

Here's a snippet of the error log demonstrating the problem:

# github.com/richardwilkes/unison/internal/skia
Error: /go/pkg/mod/github.com/richardwilkes/unison@v0.66.3/internal/skia/skia_other.go:1259:2: type [1073741824]uint16 too large
Error: /go/pkg/mod/github.com/richardwilkes/unison@v0.66.3/internal/skia/skia_other.go:1264:2: type [1073741824]uint16 too large
Error: /go/pkg/mod/github.com/richardwilkes/unison@v0.66.3/internal/skia/skia_other.go:1264:2: type [1073741824]float32 larger than address space
Error: /go/pkg/mod/github.com/richardwilkes/unison@v0.66.3/internal/skia/skia_other.go:1264:2: type [1073741824]float32 too large

These are the lines of code triggering the build failure: https://github.com/richardwilkes/unison/blob/main/internal/skia/skia_other.go#L1258-L1267

    copy(((*[1 << 30]uint16)(unsafe.Pointer(buffer.glyphs)))[:len(glyphs)], glyphs)
    copy(((*[1 << 30]float32)(unsafe.Pointer(buffer.pos)))[:len(positions)], positions)

Looks like this [1<<30]type array pattern creates arrays that are too large on 32-bit systems. I would recommend changing that to using (1<<31 - unsafe.Sizeof(type)) / unsafe.Sizeof(type) to get one array unit under the maximum allowed signed 32-bit integer bounds.

For reference, I found a similar issue with go-sqlite3 package and the recommended resolution is similar: https://github.com/mattn/go-sqlite3/issues/238

I'll work on a PR to address this and nail down the max size that should work. Thanks again!

richardwilkes commented 10 months ago

I'd be very surprised if unison could run correctly under 32-bit, since I've made the assumption everywhere that it only runs on 64-bit targets, as that's all I've needed.

Happy to look at a PR that would allow it to run on such systems... but I'm surprised anyone cares in this day and age.

JamesDunne commented 10 months ago

Agreed. I don't particularly care if it runs on 32-bit myself either. I just noticed a cross-platform build failure and thought I'd at least look into it.

JamesDunne commented 10 months ago

Ah well after fixing that array issue I came across another 64-bit assumption here:

# github.com/richardwilkes/unison
./field.go:1058:17: cannot use math.MaxInt64 (untyped int constant 9223372036854775807) as int value in argument to f.SetSelection (overflows)

Not sure how prevalent these assumptions are so I think I'll just quit while I'm ahead 😆.

Going to close this issue now. Thanks!