Skip to content

Fix export to image not capturing current view position#1784

Draft
ghostiee-11 wants to merge 2 commits into
holoviz:mainfrom
ghostiee-11:fix/export-captures-view-position
Draft

Fix export to image not capturing current view position#1784
ghostiee-11 wants to merge 2 commits into
holoviz:mainfrom
ghostiee-11:fix/export-captures-view-position

Conversation

@ghostiee-11

@ghostiee-11 ghostiee-11 commented Mar 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1773

The fix captures the current view bounds from the live panel before the export recreates the component, and ensures the export dimensions match the browser viewport instead of the spec's smaller defaults.

After fix:

vega_lite_editor

How it was tested

Uploaded a fireball dataset in Lumen AI, generated a world map with projection: equalEarth, and exported as PNG. Before the fix, the export showed only North America. After the fix, the full world map is visible matching the browser view. Also added 21 unit tests.

When exporting VegaLite charts as PNG/JPEG/SVG/PDF, the exported image
now matches the browser viewport instead of rendering at default bounds.

Three improvements:
- Capture zoom/pan bounds from the live panel selection state and
  inject them into the export spec (via selection value or scale.domain)
- Scale export dimensions to at least 1600x800 to match typical browser
  container sizes, preventing geographic map projections from cropping
- Set width/height directly on the Panel Vega pane before export so
  Panel Vega.export() uses the correct dimensions
@ghostiee-11
ghostiee-11 force-pushed the fix/export-captures-view-position branch from 2964ca4 to 83265f6 Compare March 22, 2026 07:03
@ghostiee-11
ghostiee-11 marked this pull request as ready for review March 22, 2026 07:07
@codecov

codecov Bot commented Mar 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.30%. Comparing base (461399f) to head (24a982f).
⚠️ Report is 99 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1784      +/-   ##
==========================================
+ Coverage   69.29%   69.30%   +0.01%     
==========================================
  Files         171      171              
  Lines       29249    29263      +14     
==========================================
+ Hits        20269    20282      +13     
- Misses       8980     8981       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ahuang11 ahuang11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the result is desired. However, I'm not sure this is the right implementation; seems overly complex.

Comment thread lumen/ai/editors.py Outdated
Comment on lines +310 to +317
panel = getattr(self.component, '_panel', None)
if panel is None or not hasattr(panel, 'selection') or panel.selection is None:
return None
bounds = {}
for param_name in panel.selection.param:
if param_name == 'name':
continue
value = getattr(panel.selection, param_name)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a big fan of all the getattr and hasattr

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah sure will fix it

Comment thread lumen/ai/editors.py Outdated
Comment on lines +354 to +437
@staticmethod
def _inject_encoding_domains(spec: dict, bounds: dict) -> None:
"""Fallback: inject scale.domain on matching encoding channels.

Modifies spec in place. Handles x, y, x2, y2 channels.
Does not handle longitude/latitude (they use projections, not scales).
"""
def inject_encoding(enc: dict) -> dict:
enc = dict(enc)
for channel in ('x', 'y', 'x2', 'y2'):
if channel not in enc:
continue
ch_spec = enc[channel]
field = ch_spec.get('field')
if field and field in bounds:
ch_spec = dict(ch_spec)
scale = dict(ch_spec.get('scale', {}))
scale['domain'] = list(bounds[field])
ch_spec['scale'] = scale
enc[channel] = ch_spec
return enc

if 'encoding' in spec:
spec['encoding'] = inject_encoding(spec['encoding'])
if 'layer' in spec:
spec['layer'] = [
dict(layer, encoding=inject_encoding(layer['encoding']))
if 'encoding' in layer else layer
for layer in spec['layer']
]

@staticmethod
def _apply_bounds_to_spec(spec: dict, bounds: dict) -> dict:
"""Inject zoom/pan bounds into a Vega-Lite spec.

Uses two strategies for standard x/y charts:
1. Primary: Set the value of an interval selection with bind="scales".
2. Fallback: Inject scale.domain on encoding channels directly.

For geographic maps with projections, zoom/pan state is preserved
by matching the export dimensions to the live panel dimensions
(handled in export(), not here).

Parameters
----------
spec : dict
The Vega-Lite specification.
bounds : dict
Field-name-to-[min, max] mapping from the live selection.

Returns
-------
dict
Modified spec with current view bounds applied.
"""
spec = dict(spec)

# Strategy 1: Set selection parameter value (universal approach)
if VegaLiteEditor._set_selection_value(spec, bounds):
return spec

# Also check inside layers for selection params
if 'layer' in spec:
for i, layer in enumerate(spec['layer']):
if not isinstance(layer, dict):
continue
layer = dict(layer)
if VegaLiteEditor._set_selection_value(layer, bounds):
layers = list(spec['layer'])
layers[i] = layer
spec['layer'] = layers
return spec

# Strategy 2: Fallback to scale.domain injection on encodings
VegaLiteEditor._inject_encoding_domains(spec, bounds)

# Recurse into concatenated specs
for key in ('hconcat', 'vconcat', 'concat'):
if key in spec:
spec[key] = [
VegaLiteEditor._apply_bounds_to_spec(sub, bounds)
for sub in spec[key]
]
return spec

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a way to simplify?

Comment thread lumen/ai/editors.py Outdated
Comment on lines +481 to +485
# uses them instead of its own defaults
if hasattr(panel, 'width'):
panel.width = export_width
if hasattr(panel, 'height'):
panel.height = export_height

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think all panels should have width.

@ahuang11
ahuang11 marked this pull request as draft March 24, 2026 01:56
@ghostiee-11

Copy link
Copy Markdown
Collaborator Author

Thanks for the review! You're right that the implementation is more complex than it needs to be. The core fix for #1773 is really about export dimensions not matching the browser viewport. I'll simplify by:

  • Removing all unnecessary hasattr/getattr guards
  • Simplifying or removing the bounds injection logic
  • Keeping the dimension fix clean and straightforward

I'll push a simplified version shortly.

Strip overengineered bounds-capture logic per review feedback.
The core issue was container-sized specs exporting at 800x400.
Fix uses 1600x800 viewport defaults instead. Removes all
getattr/hasattr guards, bounds injection, and recursion helpers.
@ghostiee-11
ghostiee-11 marked this pull request as ready for review March 24, 2026 16:12
@ahuang11

Copy link
Copy Markdown
Contributor

The simplification doesn't exactly capture current view; only makes it bigger?

@ahuang11
ahuang11 marked this pull request as draft March 24, 2026 17:23
@ghostiee-11

Copy link
Copy Markdown
Collaborator Author

Yup.. lemme look and refactor this according to this issue

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.

Export to image doesn't capture current view position

2 participants