Feature/available image bounds (Draft)

[vade]

Link the Issue(s) this Pull Request is related to.

Fixes #51

Summarize your change.

I wanted to open a discussion on the 'right' way of exposing in a cross platform swift native manner the image_bounds of a media reference. This is absolutely a draft and just meant to spark some discussion.

We need a standard way to bridge C++ std::optional<IMATH_NAMESPACE::Box2d> across Obj-C and into Swift.

In this draft PR, we propose adding a CxxBox.h header which exposes a simple struct CxxBox2D which is defined as follows:

typedef struct CxxBox2D {
    double minX;
    double minY;
    double maxX;
    double maxY;
} CxxBox2D;

We then add 3 new functions to opentimelineio.h / .m :

bool media_reference_available_image_bounds(CxxRetainer* self, CxxBox2D* );
void media_reference_set_available_image_bounds(CxxRetainer* self, CxxBox2D image_bounds);
void media_reference_clear_available_image_bounds(CxxRetainer* self);

which we can expose to swift MediaReference class as a property which is a SIMD4<Double> from the standard library.

Pro's - K.I.S.S. - and no reliance on CGRect or Apple platform stuff.

Con's - we dont have the benefits of either CGRect nor the Box type and its functions from IMATH.

Thoughts?

Reference associated tests.

No tests yet...

[vade]

I guess the question really becomes, do we consider it a requirement to expose the Box<Vec2 Precision> data type and its functions as first class citizens in swift?

If the answer is yes, theres work to be done to make the interface and expose it. If not, something like this might be sufficient.

[jchen9]

As a Swift/Apple platform OTIO user, I would probably prefer having CGRect-based APIs to work with.

CGRect-related data structures might have already been available in Swift Foundation in Linux runtime. I would need to double-check that though.

[vade]

hey @jchen9 I concur - if they are available, lets def use them. I didn't see them in the Swift Standard Library but I may have missed something!

[jchen9]

Yeah, they are there. I tried Swift 5.10 and 6.0.1 Swift Linux runtimes.

% uname -a
Linux jchen9 x86_64 #1 SMP PREEMPT_DYNAMIC Thu Jul 25 19:24:21 PDT 2024 x86_64 x86_64 x86_64 GNU/Linux
(jchen9) /home/jchen/Applications/Swift/swift-5.10-DEVELOPMENT-SNAPSHOT-2024-05-31-a-ubi9/usr/bin (jchen) :
% ./swift repl
error: This version of LLDB does not support DWARF version 5 or later.
warning: (x86_64) /lib64/libstdc++.so.6 No LZMA support found for reading .gnu_debugdata section
warning: (x86_64) /lib64/libm.so.6 No LZMA support found for reading .gnu_debugdata section
warning: (x86_64) /lib64/libgcc_s.so.1 No LZMA support found for reading .gnu_debugdata section
Welcome to Swift version 5.10-dev (LLVM e98989b1092ff3a, Swift 0e5cb27c42ec3c7).
Type :help for assistance.
  1> import Foundation
  2> let foo: CGRect = CGRect(
Available completions:
        CGRect( -- from: any Decoder
        CGRect( -- origin: CGPoint, size: CGSize
        CGRect( -- x: CGFloat, y: CGFloat, width: CGFloat, height: CGFloat
        CGRect( -- x: Double, y: Double, width: Double, height: Double
        CGRect( -- x: Int, y: Int, width: Int, height: Int
  2> let foo: CGRect = CGRect(x: 1, y: 2, width: 3, height: 4)
foo: Foundation.CGRect = {
  origin = {
    x = 1
    y = 2
  }
  size = {
    width = 3
    height = 4
  }
}
  3> foo.
Available completions:
        foo. -- contains(rect2: CGRect) -> Bool
        foo. -- contains(point: CGPoint) -> Bool
        foo. -- divided(atDistance: CGFloat, from: CGRectEdge) -> (slice: CGRect, remainder: CGRect)
        foo. -- encode(to: any Encoder) -> Void
        foo. -- height: CGFloat
        foo. -- insetBy(dx: CGFloat, dy: CGFloat) -> CGRect
        foo. -- integral: CGRect
        foo. -- intersection(r2: CGRect) -> CGRect
        foo. -- intersects(r2: CGRect) -> Bool
        foo. -- isEmpty: Bool
        foo. -- isInfinite: Bool
        foo. -- isNull: Bool
        foo. -- maxX: CGFloat
        foo. -- maxY: CGFloat
        foo. -- midX: CGFloat
        foo. -- midY: CGFloat
        foo. -- minX: CGFloat
        foo. -- minY: CGFloat
        foo. -- offsetBy(dx: CGFloat, dy: CGFloat) -> CGRect
        foo. -- origin: CGPoint
        foo. -- self: CGRect
        foo. -- size: CGSize
        foo. -- standardized: CGRect
        foo. -- union(r2: CGRect) -> CGRect
        foo. -- width: CGFloat
  3> foo.maxX
$R0: Foundation.CGFloat = 4
  4> foo.maxY
$R1: Foundation.CGFloat = 6
  5>

[vade]

Oh thats unexpected. Thanks for the info @jchen9 - I'll def leverage CG* for the types. Thats great news.

[furby-tm]

@jchen9 thanks for letting us know about CGRect being available in Foundation (cross-platform), are you aware the minimum Swift version that was made available in?

I propose, if it's possible, we try to target a minimum swift-tools-version of 5.9 or 5.10 (currently we are on 5.6) moving forward, as that's also the minimum for Swift/C++ interop, and I think the consensus is that we'd eventually like to deprecate the objective-c layer and make this Swift package available to Linux users as well.

We should also consider duplicating the package manifest code in a [email protected] if backwards compatibility is important.

[jchen9]

I can't find prebuilt Swift 5.9 Linux binary from Swift.org. From the history of https://github.com/swiftlang/swift-corelibs-foundation/blob/main/Sources/Foundation/NSGeometry.swift these structs might have been in the Foundation for a while.

[vade]

Hi there!

If we chose to go w/ Swift 5.9 I presume that also means we are safe to assume Foundation on all Swift supported platforms should have CGRect - therefore its ok to refactor this PR and adopt CGRect to be exposed from the API?

Give a thumbs up and I will try to find some time to work on it and close this out.

Thanks ya'll

[jminor]

Yes, that sounds good to me. See also this PR: https://github.com/OpenTimelineIO/OpenTimelineIO-Swift-Bindings/pull/61

[vade]

@jminor apologies for the delay - i pushed the changes, including a small test - i added a clip reference test OTIO sourced from the main OTIO testing resources.

[vade]

Thanks for the prompt review @jminor I cleaned up based off of your suggestions. Much obliged!