Skip to content

Conversation

@ikpil
Copy link
Owner

@ikpil ikpil commented May 14, 2025

Reduced number of global variables used in samples

Reduced number of global variables used in samples
@ikpil ikpil merged commit d61b5e5 into main May 14, 2025
7 checks passed
@ikpil ikpil deleted the pr/upstream-fixed-another-shape-distance-assert branch May 18, 2025 02:41
@ikpil
Copy link
Owner Author

ikpil commented May 19, 2025

@CodiumAI-Agent /review

@CodiumAI-Agent
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 5 🔵🔵🔵🔵🔵
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Coordinate Scaling

Inconsistent use of static s_windowScale and s_framebufferScale versus context-based values may cause misaligned input and UI scaling on high-DPI displays.

Monitor* primaryMonitor = _context.glfw.GetPrimaryMonitor();
if (null != primaryMonitor)
{
    if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
    {
        _context.glfw.GetMonitorContentScale(primaryMonitor, out s_framebufferScale, out s_framebufferScale);
    }
    else
    {
        _context.glfw.GetMonitorContentScale(primaryMonitor, out s_windowScale, out s_windowScale);
    }
Resource Lifecycle

Verify that _context.draw.Destroy() and _context.draw.Create(_context) are always paired when samples are restarted or switched to avoid graphics resource leaks.

{
    s_sample?.Dispose();
    s_sample = null;
    _context.draw.Destroy();
    DestroyUI();
}

private void OnWindowResize(Vector2D<int> resize)
{
    var width = resize.X;
    var height = resize.Y;
Input Handling

Confirm that GlfwHelpers.GetKey handles null or uninitialized context.window properly and that key state mapping remains correct when the context changes.

public static unsafe InputAction GetKey(SampleContext context, Keys key)
{
    if (null == context.glfw)
        return InputAction.Release;

    var state = context.glfw.GetKey(context.window, key);
    switch (state)
    {

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.

2 participants