-
Notifications
You must be signed in to change notification settings - Fork 0
Fix #37 and #35: Title styling, frame positioning, and unique text-style ids #44
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
1a40dfa
Fix #35: allocate unique text-style-def ids per document
leogdion c6b74c5
Fix #37: add Title styling and frame positioning modifiers
leogdion 17b1924
Fix a crash and two silent-wrong-output bugs in title positioning
leogdion aec1ed6
Namespace the decimal formatter; document why not Decimal.FormatStyle
leogdion 517a1c2
Express the FCPXML decimal formatter as String.init(fcpxmlValue:)
leogdion 1920647
Adopt typed throws across FCPKitDSL
leogdion efac192
Merge remote-tracking branch 'origin/v0.1.x' into issue/37-title-styl…
leogdion File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // | ||
| // DecimalString.swift | ||
| // FCPKit | ||
| // | ||
| // Created by Leo Dion. | ||
| // Copyright © 2026 BrightDigit. | ||
| // | ||
| // Permission is hereby granted, free of charge, to any person | ||
| // obtaining a copy of this software and associated documentation | ||
| // files (the "Software"), to deal in the Software without | ||
| // restriction, including without limitation the rights to use, | ||
| // copy, modify, merge, publish, distribute, sublicense, and/or | ||
| // sell copies of the Software, and to permit persons to whom the | ||
| // Software is furnished to do so, subject to the following | ||
| // conditions: | ||
| // | ||
| // The above copyright notice and this permission notice shall be | ||
| // included in all copies or substantial portions of the Software. | ||
| // | ||
| // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, | ||
| // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES | ||
| // OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND | ||
| // NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT | ||
| // HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, | ||
| // WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
| // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR | ||
| // OTHER DEALINGS IN THE SOFTWARE. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| /// Formats a `Double` for an FCPXML attribute, collapsing whole numbers. | ||
| /// | ||
| /// Final Cut writes `63` rather than `63.0`, so whole values lose their fractional | ||
| /// part. Values outside `Int`'s range, and non-finite values, fall back to the | ||
| /// plain `Double` description: converting them with `Int(_:)` would trap and take | ||
| /// the host process down with it. | ||
| internal func decimalString(_ value: Double) -> String { | ||
| guard value.isFinite else { | ||
| return String(value) | ||
| } | ||
| guard value.truncatingRemainder(dividingBy: 1) == 0, | ||
| value >= Double(Int.min), value <= Double(Int.max) | ||
| else { | ||
| return String(value) | ||
| } | ||
| return String(Int(value)) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,199 @@ | ||
| // | ||
| // FramePosition.swift | ||
| // FCPKit | ||
| // | ||
| // Created by Leo Dion. | ||
| // Copyright © 2026 BrightDigit. | ||
| // | ||
| // Permission is hereby granted, free of charge, to any person | ||
| // obtaining a copy of this software and associated documentation | ||
| // files (the "Software"), to deal in the Software without | ||
| // restriction, including without limitation the rights to use, | ||
| // copy, modify, merge, publish, distribute, sublicense, and/or | ||
| // sell copies of the Software, and to permit persons to whom the | ||
| // Software is furnished to do so, subject to the following | ||
| // conditions: | ||
| // | ||
| // The above copyright notice and this permission notice shall be | ||
| // included in all copies or substantial portions of the Software. | ||
| // | ||
| // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, | ||
| // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES | ||
| // OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND | ||
| // NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT | ||
| // HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, | ||
| // WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
| // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR | ||
| // OTHER DEALINGS IN THE SOFTWARE. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| /// A position within the video frame. | ||
| /// | ||
| /// Final Cut's `adjust-transform position` is expressed as a percentage of the | ||
| /// frame **height on both axes**, measured from the frame centre, with Y | ||
| /// pointing up. Alignment cases resolve without knowing the frame size; | ||
| /// absolute pixel coordinates need the enclosing sequence's format. | ||
| public struct FramePosition: Equatable, Sendable { | ||
| /// The nine standard frame alignments. | ||
| public enum Alignment: Equatable, Sendable { | ||
| /// The top-leading corner. | ||
| case topLeading | ||
| /// The top edge, horizontally centered. | ||
| case top | ||
| /// The top-trailing corner. | ||
| case topTrailing | ||
| /// The leading edge, vertically centered. | ||
| case leading | ||
| /// The frame centre. | ||
| case center | ||
| /// The trailing edge, vertically centered. | ||
| case trailing | ||
| /// The bottom-leading corner. | ||
| case bottomLeading | ||
| /// The bottom edge, horizontally centered. | ||
| case bottom | ||
| /// The bottom-trailing corner. | ||
| case bottomTrailing | ||
| } | ||
|
|
||
| /// How a position is expressed before resolution. | ||
| internal enum Kind: Equatable, Sendable { | ||
| case alignment(Alignment, inset: Double) | ||
| case absolute(x: Double, y: Double) | ||
| } | ||
|
|
||
| internal let kind: Kind | ||
|
|
||
| /// A position at a frame alignment, optionally inset in points. | ||
| public static func aligned(_ alignment: Alignment, inset: Double = 0) -> FramePosition { | ||
| FramePosition(kind: .alignment(alignment, inset: inset)) | ||
| } | ||
|
|
||
| /// A position at absolute pixel coordinates, with the origin at the top left. | ||
| public static func absolute(x: Double, y: Double) -> FramePosition { | ||
| FramePosition(kind: .absolute(x: x, y: y)) | ||
| } | ||
| } | ||
|
|
||
| extension FramePosition { | ||
| /// Converts absolute pixels (origin top-left) into Final Cut's percent-of-height units. | ||
| /// | ||
| /// The divisor is the frame **height** on both axes; that is what makes the | ||
| /// values in the 16:9 fixtures land correctly. | ||
| internal static func percent( | ||
| x absoluteX: Double, | ||
| y absoluteY: Double, | ||
| width: Double, | ||
| height: Double | ||
| ) -> (x: Double, y: Double) { | ||
| ( | ||
| x: (absoluteX - width / 2) / height * 100, | ||
| y: (height / 2 - absoluteY) / height * 100 | ||
| ) | ||
| } | ||
|
|
||
| /// Resolves an alignment, returning `nil` when it needs no transform. | ||
| /// | ||
| /// - Throws: ``BuildError/missingFrameSize`` when a non-zero inset is requested | ||
| /// without a frame size. An inset is in points, and converting points to | ||
| /// Final Cut's percent-of-height unit requires the frame height — silently | ||
| /// dropping it would emit a position the caller did not ask for. | ||
| private static func alignmentPercent( | ||
| _ alignment: Alignment, | ||
| inset: Double, | ||
| frameSize: (width: Double, height: Double)? | ||
| ) throws -> (x: Double, y: Double)? { | ||
| // `.center` is the frame centre on both axes, so an inset has no direction | ||
| // to move along and the position needs no `adjust-transform` at all. | ||
| if alignment == .center { | ||
| return nil | ||
| } | ||
|
|
||
| guard inset == 0 || frameSize != nil else { | ||
| throw BuildError.missingFrameSize | ||
| } | ||
|
|
||
| // Vertical extent is exactly ±50% of the height. Horizontal extent depends | ||
| // on the aspect ratio, because the unit's divisor is the height on both | ||
| // axes; 16:9 is assumed when no format is known. | ||
| let aspect = frameSize.map { $0.width / $0.height } ?? (16.0 / 9.0) | ||
| let halfWidth = aspect / 2 * 100 | ||
| let insetPercent = frameSize.map { inset / $0.height * 100 } ?? 0 | ||
|
|
||
| return ( | ||
| x: horizontalPercent(alignment, extent: halfWidth, inset: insetPercent), | ||
| y: verticalPercent(alignment, inset: insetPercent) | ||
| ) | ||
| } | ||
|
|
||
| /// The horizontal component of an alignment, in percent-of-height units. | ||
| private static func horizontalPercent( | ||
| _ alignment: Alignment, | ||
| extent: Double, | ||
| inset: Double | ||
| ) -> Double { | ||
| switch alignment { | ||
| case .topLeading, .leading, .bottomLeading: | ||
| return -extent + inset | ||
| case .topTrailing, .trailing, .bottomTrailing: | ||
| return extent - inset | ||
| case .top, .center, .bottom: | ||
| return 0 | ||
| } | ||
| } | ||
|
|
||
| /// The vertical component of an alignment, in percent-of-height units. | ||
| private static func verticalPercent(_ alignment: Alignment, inset: Double) -> Double { | ||
| switch alignment { | ||
| case .topLeading, .top, .topTrailing: | ||
| return 50 - inset | ||
| case .bottomLeading, .bottom, .bottomTrailing: | ||
| return -50 + inset | ||
| case .leading, .center, .trailing: | ||
| return 0 | ||
| } | ||
| } | ||
| } | ||
|
|
||
| extension FramePosition { | ||
| /// Resolves this position into an `adjust-transform position` value. | ||
| /// | ||
| /// - Parameter frameSize: The enclosing sequence's frame size, when known. | ||
| /// - Returns: The formatted `"x y"` pair, or `nil` when the position is the | ||
| /// frame centre and therefore needs no `adjust-transform` at all. | ||
| /// - Throws: ``BuildError/missingFrameSize`` when absolute coordinates were | ||
| /// used without an enclosing format. | ||
| internal func resolve(frameSize: (width: Double, height: Double)?) throws -> String? { | ||
| let point: (x: Double, y: Double) | ||
|
|
||
| switch kind { | ||
| case .alignment(let alignment, let inset): | ||
| guard | ||
| let resolved = try Self.alignmentPercent(alignment, inset: inset, frameSize: frameSize) | ||
| else { | ||
| return nil | ||
| } | ||
| point = resolved | ||
|
|
||
| case .absolute(let absoluteX, let absoluteY): | ||
| guard let frameSize else { | ||
| throw BuildError.missingFrameSize | ||
| } | ||
| point = Self.percent( | ||
| x: absoluteX, | ||
| y: absoluteY, | ||
| width: frameSize.width, | ||
| height: frameSize.height | ||
| ) | ||
| } | ||
|
|
||
| return "\(format(point.x)) \(format(point.y))" | ||
| } | ||
|
|
||
| /// Formats a component, collapsing whole numbers (`0`, not `0.0`). | ||
| fileprivate func format(_ value: Double) -> String { | ||
| decimalString(value) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in
aec1ed6, on both points.1. Global function. Now
AttributeValue.decimal(_:), file renamed to match. Applied module-wide —storyItems/anchoredItem(s)on #43 andsoftPromoteon #42 got the same treatment, so FCPKitDSL has no top-level functions left.2.
Decimal.FormatStyle— I tested it, and it's the wrong tool here. It does collapse whole numbers natively, which is the one thing it would buy us, but three problems:decimalStringsUseAPeriodRegardlessOfLocaleso nobody swaps a locale-aware formatter back in.7.777777777→7.777778. Fine for a font size, silently lossy for a position component.Decimal(Double.infinity)traps, so it doesn't even solve the crash that motivated the helper.All three verified by running them rather than reasoning about them. The rationale is in the doc comment so this doesn't get re-litigated later.
If you'd still prefer it for the font-size case specifically — where precision and range are not really at issue — say the word and I'll split the two call sites. My read is that one formatter with one rule is easier to reason about than two.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in
517a1c2— went with theStringinit.Both call sites produce a
Stringfrom aDouble, which is what an init is for, and it keeps the conversion attached to the type being produced rather than hanging a domain-specific property offDouble. Call sites read asString(fcpxmlValue: fontSize).One thing worth flagging: the argument label is load-bearing.
String(_: Double)already exists viaLosslessStringConvertible, so an unlabelled init would have quietly overloaded it — anyone writingString(someDouble)would have silently got the FCPXML formatter instead of the stdlib one. I verified the labelled version coexists cleanly:String(63.5)still routes to the stdlib and returns"63.5".Behaviour unchanged — same guards, same fallbacks, same tests, and the exported deck is byte-identical.