Skip to content
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

intrinsics.objc_send behaves differently than proc with @objc_type and @objc_name #2121

Closed
jceipek opened this issue Oct 9, 2022 · 8 comments
Labels

Comments

@jceipek
Copy link
Contributor

jceipek commented Oct 9, 2022

I expect MTK.View_setClearColor(mtkView, bgCol) to be the same as mtkView->setClearColor(bgCol) to be the same as intrinsics.objc_send(nil, mtkView, "setClearColor:", bgCol), but the runtime behavior seems to differ.

Context

        Odin: dev-2022-10-nightly:775c9648
        OS:   macOS Monterey 12.5 (build: 21G115, kernel: 21.6.0)
        CPU:  Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
        RAM:  32768 MiB 

main.odin program:

package main

@require foreign import "system:Cocoa.framework"
@require foreign import "system:AppKit.framework"
import NS "vendor:darwin/Foundation"
import MTL "vendor:darwin/Metal"
import MTK "vendor:darwin/MetalKit"

import "core:fmt"
import "core:intrinsics"

main :: proc() {
    device := MTL.CreateSystemDefaultDevice()
    mtkView := MTK.View.alloc()
    bgCol := MTL.ClearColor{0.1,0.2,0.3,0.4}
    fmt.printf("bgCol: %v\n", bgCol)
    { // A
        intrinsics.objc_send(nil, mtkView, "setClearColor:", bgCol)
        actualCol := MTK.View_clearColor(mtkView)
        fmt.printf("A: %v after intrinsics.objc_send(nil, mtkView, \"setClearColor:\", bgCol)\n", actualCol)
    }
    { // B
        MTK.View_setClearColor(mtkView, bgCol)
        actualCol := MTK.View_clearColor(mtkView)
        fmt.printf("B: %v after MTK.View_setClearColor(mtkView, bgCol)\n", actualCol)
    }    
    { // C
        mtkView->setClearColor(bgCol)
        actualCol := MTK.View_clearColor(mtkView)
        fmt.printf("C: %v after mtkView->setClearColor(bgCol)\n", actualCol)
    }    
    { // D
        intrinsics.objc_send(nil, mtkView, "setClearColor:", bgCol)
        actualCol := intrinsics.objc_send(MTL.ClearColor, mtkView, "clearColor")
        fmt.printf("D: %v after intrinsics.objc_send(nil, mtkView, \"setClearColor:\", bgCol)\n", actualCol)
    }
    { // E
        MTK.View_setClearColor(mtkView, bgCol)
        actualCol := intrinsics.objc_send(MTL.ClearColor, mtkView, "clearColor")
        fmt.printf("E: %v after MTK.View_setClearColor(mtkView, bgCol)\n", actualCol)
    }    
    { // F
        mtkView->setClearColor(bgCol)
        actualCol := intrinsics.objc_send(MTL.ClearColor, mtkView, "clearColor")
        fmt.printf("F: %v after mtkView->setClearColor(bgCol)\n", actualCol)
    }    
}

Expected Behavior

The program should output:

bgCol: ClearColor{red = 0.100, green = 0.200, blue = 0.300, alpha = 0.400}
A: ClearColor{red = 0.100, green = 0.200, blue = 0.300, alpha = 0.400} after intrinsics.objc_send(nil, mtkView, "setClearColor:", bgCol)
B: ClearColor{red = 0.100, green = 0.200, blue = 0.300, alpha = 0.400} after MTK.View_setClearColor(mtkView, bgCol)
C: ClearColor{red = 0.100, green = 0.200, blue = 0.300, alpha = 0.400} after mtkView->setClearColor(bgCol)
D: ClearColor{red = 0.100, green = 0.200, blue = 0.300, alpha = 0.400} after intrinsics.objc_send(nil, mtkView, "setClearColor:", bgCol)
E: ClearColor{red = 0.100, green = 0.200, blue = 0.300, alpha = 0.400} after MTK.View_setClearColor(mtkView, bgCol)
F: ClearColor{red = 0.100, green = 0.200, blue = 0.300, alpha = 0.400} after mtkView->setClearColor(bgCol)

Current Behavior

The program outputs:

bgCol: ClearColor{red = 0.100, green = 0.200, blue = 0.300, alpha = 0.400}
A: ClearColor{red = 0.000, green = -11259617436080485908895017282378495051889730797178800267793906676224359855271427702780.000, blue = -0.000, alpha = -0.000} after intrinsics.objc_send(nil, mtkView, "setClearColor:", bgCol)
B: ClearColor{red = 0.000, green = 0.000, blue = 0.000, alpha = 0.100} after MTK.View_setClearColor(mtkView, bgCol)
C: ClearColor{red = 0.000, green = 0.000, blue = 0.000, alpha = 0.100} after mtkView->setClearColor(bgCol)
D: ClearColor{red = 0.000, green = -11259617436080485908895017282378495051889730797178800267793906676224359855271427702780.000, blue = 0.000, alpha = 0.000} after intrinsics.objc_send(nil, mtkView, "setClearColor:", bgCol)
E: ClearColor{red = 0.000, green = 0.000, blue = 0.000, alpha = 0.100} after MTK.View_setClearColor(mtkView, bgCol)
F: ClearColor{red = 0.000, green = 0.000, blue = 0.000, alpha = 0.100} after mtkView->setClearColor(bgCol)

Failure Information (for bugs)

Note that A and D print different output than B,C,E, and F. A and D do not seem to correlate with bgCol at all. B, C, E, and F have the red channel in the alpha channel and have 0 values in the other channels.

Test Expected Actual (varies per run)
A 0.1 0.2 0.3 0.4 0.0 -11259617436080485908895017282378495051889730797178800267793906676224359855271427702780.000 -0.0 -0.0
B 0.1 0.2 0.3 0.4 0.0 0.0 0.0 0.1
C 0.1 0.2 0.3 0.4 0.0 0.0 0.0 0.1
D 0.1 0.2 0.3 0.4 0.0 -11259617436080485908895017282378495051889730797178800267793906676224359855271427702780.000 0.0 -0.0
E 0.1 0.2 0.3 0.4 0.0 0.0 0.0 0.1
F 0.1 0.2 0.3 0.4 0.0 0.0 0.0 0.1

This Objective-C program works as expected:

#import <Foundation/Foundation.h>
#import <Metal/Metal.h>
#import <MetalKit/MetalKit.h>
#import <QuartzCore/QuartzCore.h>

int main(int argc, const char * argv[]) {
    MTKView *view = [MTKView alloc];
    MTLClearColor color = MTLClearColorMake(0.1, 0.2, 0.3, 0.4);
    [view setClearColor:color];
    NSLog(@"%f %f %f %f", [view clearColor].red,
          [view clearColor].green,
          [view clearColor].blue,
          [view clearColor].alpha); // 0.100000 0.200000 0.300000 4.000000, as expected
    return 0;
}

Steps to Reproduce

  1. Create a folder called debug containing a file main.odin with the contents of the test program above.
  2. Compile from that folder with $ odin build . -out:test or $ odin build . -debug -out:test
  3. Run the program with $ ./test. Note that the output for A and D varies between runs.
@gingerBill gingerBill added the bug label Oct 10, 2022
@gingerBill
Copy link
Member

This appears to be an ABI related bug

gingerBill added a commit that referenced this issue Oct 10, 2022
@gingerBill
Copy link
Member

Could you give this another go on the latest commit please, @jceipek?

@Lperlind
Copy link
Contributor

I had this issue too. Although I was on an m1 Mac and once I compiled to arm64 things were working.

It may be important to test an arm64 build of the fix in case of regressions.

Maybe add the example above to the test/issues folder

@gingerBill
Copy link
Member

@Lperlind One issue is that we cannot test arm64 with GitHub CI. So the tests would have to only be odin check ... -target:darwin_arm64 or odin build ... -target:darwin_arm64 -build-mode:object equivalent.

@Lperlind
Copy link
Contributor

Okay, I guess I can manually verify the change in a bit on arm64.

@jceipek
Copy link
Contributor Author

jceipek commented Oct 10, 2022

Could you give this another go on the latest commit please, @jceipek?

I did, and it works as expected! It also seems to eliminate the need for the hack here:

@(objc_type=Window, objc_name="initWithContentRect")
Window_initWithContentRect :: proc (self: ^Window, contentRect: Rect, styleMask: WindowStyleMask, backing: BackingStoreType, doDefer: bool) -> ^Window {
self := self
// HACK: due to a compiler bug, the generated calling code does not
// currently work for this message. Has to do with passing a struct along
// with other parameters, so we don't send the rect here.
// Omiting the rect argument here actually works, because of how the C
// calling conventions are defined.
self = msgSend(^Window, self, "initWithContentRect:styleMask:backing:defer:", styleMask, backing, doDefer)
// apply the contentRect now, since we did not pass it to the init call
msgSend(nil, self, "setContentSize:", contentRect.size)
msgSend(nil, self, "setFrameOrigin:", contentRect.origin)
return self
}
-- I think you can revert ad98efe

Thanks!

@Lperlind
Copy link
Contributor

I can confirm arm64 is fine too. And running the x64 code with rosetta also passes.

@gingerBill
Copy link
Member

If this is fixed, I'll close the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants