Skip to content

Fix texture cache with viewport tilt#3867

Open
TrueDoctor wants to merge 3 commits intomasterfrom
fix-texture-cache-tilt
Open

Fix texture cache with viewport tilt#3867
TrueDoctor wants to merge 3 commits intomasterfrom
fix-texture-cache-tilt

Conversation

@TrueDoctor
Copy link
Member

No description provided.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves an issue where the texture cache did not correctly handle tilted viewports, leading to rendering inaccuracies. The changes introduce a more robust method for calculating viewport bounds, enhance the cache key to incorporate zoom and rotation, and refactor the underlying tile and region processing logic. These modifications ensure that the rendering pipeline accurately accounts for viewport transformations, providing a consistent visual experience across different orientations and zoom levels.

Highlights

  • Accurate Viewport Bounding Box Calculation: The method for calculating the viewport's bounding box in local space has been updated to consider all four corners of the viewport, ensuring correct bounds even when the viewport is rotated. Previously, it only considered two points, which is insufficient for tilted viewports.
  • Enhanced Cache Key for Viewport State: The CacheKey structure now explicitly includes device_scale, zoom, and a quantized rotation. This change allows the texture cache to differentiate between various viewport states, preventing incorrect cache hits and ensuring proper rendering when the viewport is tilted or zoomed.
  • Refactored Tile and Region Management: Several functions related to tile and region management (scene_bounds_to_tiles, tile_to_scene_bounds, tiles_to_scene_bounds, group_into_regions, split_oversized_region) have been refactored. They now operate without directly passing a scale parameter, simplifying the logic and aligning with the new approach of handling scale and rotation through the CacheKey and explicit transform parameters.
  • Updated Render and Composite Logic: The render_missing_region and composite_cached_regions functions have been updated to accept viewport_transform, viewport_origin_offset, and physical_tile_size. This ensures that individual regions are rendered and composited correctly within the overall viewport, accurately reflecting the current rotation and scale.
Changelog
  • node-graph/libraries/core-types/src/transform.rs
    • Updated viewport_bounds_in_local_space to calculate the bounding box by transforming all four corners of the viewport.
  • node-graph/nodes/gstd/src/render_cache.rs
    • Imported DAffine2 from glam for affine transformations.
    • Removed the scene_bounds field from the CachedRegion struct.
    • Modified the CacheKey struct to replace scale with device_scale, zoom, and rotation.
    • Updated the CacheKey::new constructor to accept and store device_scale, zoom, and a quantized rotation.
    • Removed the current_scale field from TileCacheImpl.
    • Removed scene_bounds and scale fields from the RenderRegion struct.
    • Renamed scene_bounds_to_tiles to bounds_to_tiles and removed its scale parameter.
    • Renamed tile_scene_start to tile_bounds and removed its scale parameter.
    • Renamed tile_to_scene_bounds to tile_bounds and removed its scale parameter.
    • Renamed tiles_to_scene_bounds to tiles_bounds and removed its scale parameter.
    • Updated TileCacheImpl::query and TileCache::query to no longer require a scale parameter.
    • Modified group_into_regions and split_oversized_region to remove scale parameters and adapt to the new RenderRegion structure.
    • Refactored render_output_cache to use the new CacheKey fields, calculate viewport_pixel_bounds, and pass updated parameters to tile_cache.query, render_missing_region, and composite_cached_regions.
    • Updated render_missing_region function signature and internal logic to use viewport_transform, viewport_origin_offset, and physical_tile_size for accurate region rendering and metadata transformation.
    • Updated composite_cached_regions function signature and internal logic to use physical_tile_size, device_scale, viewport_origin_offset, and viewport_transform for correct region compositing and metadata merging.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@TrueDoctor TrueDoctor force-pushed the fix-texture-cache-tilt branch from 39f9685 to 83c25fb Compare March 8, 2026 14:07
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the texture cache to support viewport rotation. Key changes include updating viewport_bounds_in_local_space in core-types to correctly calculate the bounding box for rotated transforms, extending the CacheKey in render_cache to include zoom and rotation for proper cache invalidation, and updating the tiling and compositing logic to handle these new transform components. The changes appear to correctly implement caching for rotated viewports. The suggestion to extract the magic number into a named constant remains valid to improve maintainability.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

ctx.try_pointer_position(),
);

let physical_tile_size = (TILE_SIZE as f64 * device_scale).round() as u32;
Copy link
Member

Choose a reason for hiding this comment

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

I assume this means vello tiles are assumed to be the incorrect size when device scale is not 1? vello always renders in tiles of 256 in device pixels right? The preference Render Tile Resolution should be in device pixels right? scale seem to have influence on that currently?

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