Skip to content

fix: use double instead of CGFloat for mjpegScalingFactor public API#1168

Open
AsijitM wants to merge 1 commit into
appium:masterfrom
AsijitM:fix-import-coregraphics-fbconfiguration
Open

fix: use double instead of CGFloat for mjpegScalingFactor public API#1168
AsijitM wants to merge 1 commit into
appium:masterfrom
AsijitM:fix-import-coregraphics-fbconfiguration

Conversation

@AsijitM

@AsijitM AsijitM commented Jul 6, 2026

Copy link
Copy Markdown

Problem

FBConfiguration.h exposed CGFloat in its public interface:

+ (CGFloat)mjpegScalingFactor;
+ (void)setMjpegScalingFactor:(CGFloat)scalingFactor;

CGFloat is a CoreGraphics type, so the header depended on CoreGraphics even though it only imports
Foundation (it compiled only via a transitive Foundation → NSGeometry → CoreGraphics chain).

Fix (updated per review)

Rather than import CoreGraphics to prop up a CGFloat public API, switch the public getter/setter — and the
backing static — to double. CGFloat stays out of the public settings surface entirely, and the header
becomes self-contained with no CoreGraphics dependency.

+ (double)mjpegScalingFactor;
+ (void)setMjpegScalingFactor:(double)scalingFactor;

No behaviour change (CGFloat is double on 64-bit). Files: FBConfiguration.{h,m}.

@AsijitM AsijitM force-pushed the fix-import-coregraphics-fbconfiguration branch from 50fa05c to 8285ba5 Compare July 6, 2026 15:16
@mykola-mokhnach

Copy link
Copy Markdown

I would rather prefer to explicitly use Double as type for settings. CGFloat is an internal implementation detail

`FBConfiguration.h` exposed `mjpegScalingFactor` / `setMjpegScalingFactor:` using
`CGFloat`. CGFloat is a CoreGraphics implementation detail and shouldn't leak into
the public settings API; it also forced the header to depend on CoreGraphics (the
header only imports Foundation).

Switch the public getter/setter (and the backing static) to `double`, so the header
is self-contained without a CoreGraphics import and the settings surface uses a plain
scalar type. No behavior change (CGFloat is `double` on 64-bit).
@AsijitM AsijitM force-pushed the fix-import-coregraphics-fbconfiguration branch from 8285ba5 to de63ba7 Compare July 7, 2026 05:07
@AsijitM AsijitM changed the title fix: import CoreGraphics in FBConfiguration.h since it declares CGFloat APIs fix: use double instead of CGFloat for mjpegScalingFactor public API Jul 7, 2026
@AsijitM

AsijitM commented Jul 7, 2026

Copy link
Copy Markdown
Author

Good call — updated. Instead of importing CoreGraphics, I switched mjpegScalingFactor/setMjpegScalingFactor: (and the backing static) to double, so CGFloat no longer leaks into the public settings API and the header no longer needs CoreGraphics. No behaviour change (CGFloat == double on 64-bit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants