Skip to content

Get third party project "zigimg" working successfully with self-hosted. #12652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mlarouche opened this issue Aug 27, 2022 · 2 comments
Closed
Labels
bug Observed behavior contradicts documented or intended behavior
Milestone

Comments

@mlarouche
Copy link
Contributor

mlarouche commented Aug 27, 2022

Zig Version

0.10.0-dev.3757+ae8d26a6a

Steps to Reproduce

  1. git clone https://github.com/zigimg/zigimg zigimg
  2. git clone https://github.com/zigimg/test-suite test-suite
  3. cd zigimg
  4. git checkout stage2_compat
  5. Remove zig_backend check in tests/image_test.zig line 350-352, test "Test Colorf32 iterator". There is another similar test skipped as well in tests/octree_quantizer_test.zig line 34-36
  6. Do zig build test to run the test-suite

Expected Behavior

All the tests should pass.

Actual Behavior

Test [101/106] test.Test Colorf32 iterator... thread 23952 panic: switch on corrupt value
C:\Programmation\Zig\zigimg\zigimg\src\color.zig:809:66: 0x7ff7a6c6484b in next (zigimgtest.exe.obj)
            .rgba64 => |data| data[self.current_index].toColorf32(),
                                                                 ^
C:\Programmation\Zig\zigimg\zigimg\tests\image_test.zig:373:19: 0x7ff7a6c64468 in test.Test Colorf32 iterator (zigimgtest.exe.obj)
    while (it.next()) |actual| {
                  ^
C:\Programmation\Zig\zig\stage3\lib\zig\test_runner.zig:79:28: 0x7ff7a6c2165a in main (zigimgtest.exe.obj)
        } else test_fn.func();
                           ^
C:\Programmation\Zig\zig\stage3\lib\zig\std\start.zig:347:41: 0x7ff7a6c21277 in WinStartup (zigimgtest.exe.obj)
    std.debug.maybeEnableSegfaultHandler();

Something is corrupting the color iterator and the test fails. While looking at disassembly I found something weird (copied by hand from VS Code disassembly view)

374:               const expected = expectedColors[i];

lea rcx, [rbp+200h]
lea rdx, [rbp+0B0h]
mov r8d, 80h                  # This is the weird part, why it is trying to copy 128 bytes for a 16 bytes struct?
call memcpy

Contact BaroqueLarouche on zig Discord if you need any assistance with the setup. Thanks :) le

@mlarouche mlarouche added the bug Observed behavior contradicts documented or intended behavior label Aug 27, 2022
@andrewrk andrewrk added this to the 0.10.1 milestone Oct 15, 2022
@mlarouche
Copy link
Contributor Author

I found a workaround but the core of the issue is #12638.

pub fn iterator(self: Image) color.PixelStorageIterator {
    return color.PixelStorageIterator.init(&self.pixels);
}

@sizeof(Image) is 104 bytes

If Image is passed by copy, I am taking a pointer to a temporary value. Before on stage1, Image was passed by *const pointer behind the scene and never caused an issue.

@andrewrk
Copy link
Member

andrewrk commented Oct 15, 2022

Note that the snippet is indeed a bug because while zig is supposed to make choices according to perf, it is technically allowed to pass the arg either way. So passing as *const Image is not only a workaround, but a bug fix in zigimg:

pub fn iterator(self: *const Image) color.PixelStorageIterator {
    return color.PixelStorageIterator.init(&self.pixels);
}

@andrewrk andrewrk modified the milestones: 0.10.1, 0.10.0 Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

No branches or pull requests

2 participants