Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add default material::scatter() implementation #1455

Merged
merged 2 commits into from
Mar 23, 2024
Merged

Conversation

hollasch
Copy link
Collaborator

The code mixes up material::scatter() with and without a default implementation. This causes problems as the diffuse_light (derived from the material class) first defines its own no-op implementation, but later the signature of material::scatter() changes without associated updates to the diffuse_light class.

All of this is fixed by giving material::scatter() a no-op default implementation, which diffuse_light just uses implicitly. Thus, when the signature is changed, diffuse_light requires no updates.

Resolves #1379

The code mixes up `material::scatter()` with and without a default
implementation. This causes problems as the `diffuse_light` (derived
from the `material` class) first defines its own no-op implementation,
but later the signature of `material::scatter()` changes without
associated updates to the `diffuse_light` class.

All of this is fixed by giving `material::scatter()` a no-op default
implementation, which `diffuse_light` just uses implicitly. Thus, when
the signature is changed, `diffuse_light` requires no updates.

Resolves #1379
@hollasch hollasch added this to the v4.0.0 milestone Mar 22, 2024
@hollasch hollasch self-assigned this Mar 22, 2024
@hollasch hollasch requested a review from a team March 22, 2024 00:46
@hollasch hollasch modified the milestones: v4.0.0, v4.0.0-alpha.2 Mar 22, 2024
@hollasch hollasch merged commit aea024e into dev Mar 23, 2024
@hollasch hollasch deleted the default-material-scatter branch March 23, 2024 20:51
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