Sphere billboard (Impostor)#1037
Conversation
7707f89 to
6eabff0
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds billboard-based sphere impostor functionality to the Fury rendering library, providing a performance alternative to geometry-based spheres using shader techniques and camera-facing quads.
- Implements billboard rendering system with WGSL shaders that always face the camera
- Adds sphere impostor mode that renders 3D-looking spheres using 2D quads with ray-sphere intersection
- Integrates billboard functionality into existing sphere actor with an
impostorflag
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| fury/wgsl/billboard_sphere_render.wgsl | WGSL shader for rendering sphere impostors with lighting and depth |
| fury/wgsl/billboard_render.wgsl | Basic WGSL shader for camera-facing billboard quads |
| fury/tests/test_billboard.py | Comprehensive test suite for billboard and sphere impostor functionality |
| fury/shader.py | Billboard shader classes for rendering pipeline integration |
| fury/material.py | Material classes for billboard and sphere impostor rendering |
| fury/actor/curved.py | Modified sphere function to support impostor mode |
| fury/actor/billboard.py | Core billboard implementation with factory functions |
| fury/actor/init.pyi | Type stub updates for billboard exports |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| scales = radii | ||
| directions = (1, 0, 0) | ||
|
|
||
| vertices, faces = fp.prim_sphere(phi=phi, theta=theta) | ||
| return actor_from_primitive( | ||
| obj = actor_from_primitive( |
There was a problem hiding this comment.
The variable scales is set to radii but should be set to radii_arr for consistency with the processed array data used elsewhere in the function.
maharshi-gor
left a comment
There was a problem hiding this comment.
Thank you for the work @m-agour !
Let's fix some simple changes in the sphere method.
fdeb9ae to
fffe69b
Compare
Thank you @maharshi-gor, updated are made! |
|
Great work. |
fbc4532 to
882c804
Compare
|
What is the update on this @m-agour ? can you fix conflict and address Elef comments |
|
@m-agour thanks for putting this together! Here are some comments/suggestions:
In addition to this, is there a way to merge or extend |
e46ee44 to
327387a
Compare
|
Hi @maharshi-gor ready for review |
maharshi-gor
left a comment
There was a problem hiding this comment.
Thank you for this work @m-agour . Please look at the provided comments on the PR.
fury/actor/billboard.py
Outdated
| import numpy as np | ||
|
|
||
| from fury.actor import Mesh | ||
| from fury.actor.core import Mesh |
There was a problem hiding this comment.
| from fury.actor.core import Mesh | |
| from fury.actor import Mesh |
fury/actor/billboard.py
Outdated
|
|
||
| __all__ = ["billboard", "billboard_sphere", "Billboard"] |
There was a problem hiding this comment.
| __all__ = ["billboard", "billboard_sphere", "Billboard"] |
There was a problem hiding this comment.
Let's add them to __init__.pyi, Not required here.
fury/actor/billboard.py
Outdated
| stored on ``billboard_sizes`` and reused by shaders for impostor variants. | ||
| """ | ||
|
|
||
| pass |
There was a problem hiding this comment.
Let's make billbord_count, billboard_centers and billboard_sizes as properties in the Billboard actor so it stays consistent across. If someone tries to use BillBoard actor directly without _create_billboard_actor method.
There was a problem hiding this comment.
There a few more on the sphere actor, I would leave it to you how to best represent them. If you want the user to use those properties. They should be available in the object definition.
fury/actor/__init__.pyi
Outdated
| ] | ||
|
|
||
| from .billboard import billboard | ||
| from .billboard import billboard, billboard_sphere # noqa: F401 |
There was a problem hiding this comment.
No need for #noqa: F401
fury/actor/billboard.py
Outdated
| Tuple containing the configured | ||
| :class:`~fury.shader.BillboardSphereShader`. | ||
| """ | ||
| from fury.shader import BillboardSphereShader |
There was a problem hiding this comment.
Please do not import like this. Move this import on top of the file.
fury/actor/billboard.py
Outdated
| @@ -126,3 +231,23 @@ def register_billboard_render_function(wobject): | |||
| from fury.shader import BillboardShader | |||
fury/wgsl/billboard_render.wgsl
Outdated
| // Load color if available - colors are duplicated 6x like positions | ||
| let color = load_s_colors(billboard_index * 6); | ||
| varyings.color = vec4<f32>(color, 1.0); | ||
| varyings.texcoord_vert = vec2<f32>(tex_coord); |
327387a to
2096f22
Compare
|
Hi @maharshi-gor, thanks for the review, I have them all addressed! also added impostor sphere to tru by default. |
2096f22 to
a792212
Compare
a792212 to
440cbcb
Compare
Hi @guaje , sorry for the late response. I completely missed your review and that’s on me. |
|
Thanks @m-agour for the great work. LGTM Merging! |
Adding first billboard-based actor to Fury (based on #1036). It uses the lighing system and have depth almost as normal gemetry-based sphere. I put it under the sphere actor with a flag
imposter(name is up to change).Here is a super cool video of the super cool impostor:
2025-09-28.13-30-23.mp4
Update (fixed light direction; compare with previoue old video):
2025-09-29.15-44-19.mp4
it looks real to me
example: