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

Added max to bitmap and checkerboard textures, added tests for both m… #719

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Microno95
Copy link
Contributor

Description

Adds implementations of max for the bitmap and checkerboard textures, adds python binding and mean/max tests for vectorised variants. Fixes #707

Testing

Ran entire python test suite in Release mode.

Checklist

  • My code follows the style guidelines of this project
  • My changes generate no new warnings
  • My code also compiles for cuda_* and llvm_* variants. If you can't test this, please leave below
  • I have commented my code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I cleaned the commit history and removed any "Merge" commits
  • I give permission that the Mitsuba 3 project may redistribute my contributions under the terms of its license

Copy link
Member

@njroussel njroussel left a comment

Choose a reason for hiding this comment

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

I have a few comments (they are also true for the rebuild_internals method).

The suggestions might not actually be applicable/reasonable for your initial use case (volumes), I hadn't considered this when you opened the issue. I don't want to discourage using the Texture interface for volumes, but I don't think the Bitmap plugin is the appropriate choice.

src/textures/bitmap.cpp Outdated Show resolved Hide resolved
src/textures/bitmap.cpp Outdated Show resolved Hide resolved
@Microno95
Copy link
Contributor Author

Hey @njroussel, have you had a chance to read any of my comments?

@Microno95 Microno95 force-pushed the feature/textures-max-implementation branch from cf1d6b1 to dfc9d21 Compare October 1, 2023 22:47
@Microno95
Copy link
Contributor Author

I've updated the implementation to use the luminance rather than the component-wise maximum for both the RGB and Spectral cases.

…ean and max calculations in vectorised variants
@Microno95 Microno95 force-pushed the feature/textures-max-implementation branch from dfc9d21 to 046b414 Compare October 2, 2023 09:12
@wjakob wjakob force-pushed the master branch 3 times, most recently from 3f3b8d0 to 1bdea6e Compare October 12, 2023 19:49
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.

Bitmap and Checkerboard - max() Method Not Implemented
2 participants