Skip to content

feat: implement type-safe isomorphic coords helpers#1233

Open
shadowusr wants to merge 1 commit intotestplane@9from
users/shadowusr/TESTPLANE-672.update-composite-image-renderer
Open

feat: implement type-safe isomorphic coords helpers#1233
shadowusr wants to merge 1 commit intotestplane@9from
users/shadowusr/TESTPLANE-672.update-composite-image-renderer

Conversation

@shadowusr
Copy link
Copy Markdown
Member

What's done?

Implemented a type system for type-safe coords operations.

With these, it becomes much harder to confuse numbers from different spaces or units.

These helpers are broadly used in the upcoming PRs.

For example:

  • You can't get intersection of rects with different pixelRatios
  • You can't combine two coords, if one is relative to viewport and the other is relative to the whole page
  • You can't get a sum of left and top values without explicit type casting

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97b73c48d6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

scaled[key] = v[key] * pixelRatio;
}

return (pixelRatio % 1 === 0 ? scaled : roundCoords(scaled as Rect<Space, Unit>)) as unknown as CssToDevice<T>;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle Size inputs in fractional css→device conversion

fromCssToDevice explicitly accepts Size<"css">, but in the fractional-pixelRatio branch it routes through roundCoords, which only fills fields when top/left exist. For a size object ({ width, height }), this returns an empty object at runtime, so downstream code gets undefined dimensions on non-integer DPR values (for example 1.25/1.5), breaking size-based geometry.

Useful? React with 👍 / 👎.

coordRelativeToViewport: Coord<"viewport", U, "y">,
captureAreaTopRelativeToViewport: Coord<"viewport", U, "y">,
): Coord<"capture", U, "y"> => {
return getHeight(coordRelativeToViewport, captureAreaTopRelativeToViewport) as number as Coord<"capture", U, "y">;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve sign when converting viewport to capture coords

fromViewportToCaptureArea uses getHeight, which applies Math.abs, so it always returns a non-negative offset. Coordinate conversion should preserve direction (it should be the inverse of fromCaptureAreaToViewport), and inputs above the capture-area top should produce negative values; with the current code those values are mirrored to positive and can place/crop geometry on the wrong side.

Useful? React with 👍 / 👎.

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.

1 participant