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

Color-based, per-material emittance mapping #1627

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

JustinTimeCuber
Copy link
Contributor

NOTE: this PR depends on/includes the changes from #1621, so it is currently marked as a draft.
One useful feature Chunky is currently missing is the ability to differentiate between the parts of blocks that should emit light and the parts that shouldn't. Here is an example with furnaces:
image
As can be seen, the entirety of the blocks are emitting light despite the fact that physically, only the "flame" portions should be providing light.
Ideally this could be done with PBR maps, but besides the fact that we don't have those yet, that basically requires either us to provide them for every emitter (potentially a lot of work) or users to provide them (not really an ideal solution in my opinion). This PR provides 3 methods for determining how much light should be emitted from each pixel as a function of pixel color rather than an emittance map texture. One of these is REFERENCE_COLORS, which allows for specific color range(s) to be specified which should emit light while all other colors will not do so. In this mode, the strength of the emitted light is also dependent on the HSV Value of the pixel color raised to some exponent (default: 1.5). This mode is well-suited to furnaces, as well as some other blocks like lanterns, torches, glow berries, and end rods. There is currently no GUI for adjusting the reference colors, but they can be specified in JSON format and defaults are included for the aforementioned emitters.
image
For certain other emitters like magma blocks, sea lanterns, and redstone lamps, the darker pixels should emit less light, but not none. In this case, a simpler method (BRIGHTEST_CHANNEL) is used, which simply takes the HSV value raised to the same exponent and uses that as the relative emittance of that pixel:
image
There is also one more method included called (INDEPENDENT_CHANNELS) which raises each individual RGB component to the provided exponent. This isn't the default because it tends to increase the saturation of emitted light at higher powers making it a bit less realistic, but it is still included as an option for more artistic freedom.
image

The current version of this PR doesn't include a GUI option for setting REFERENCE_COLORS which can only be customized by editing the scene JSON, but everything else is customizable through the GUI. I'll look into adding a GUI option (let me know if this is needed before merging) based on the layered fog GUI in #1618.

@leMaik
Copy link
Member

leMaik commented Sep 19, 2023

We can add a GUI in a future PR.

@Peregrine05
Copy link
Member

How easily is this extended to other material properties?

@JustinTimeCuber
Copy link
Contributor Author

JustinTimeCuber commented Oct 26, 2023 via email

@leMaik leMaik force-pushed the emittance-mapping branch from ba268a7 to eba5b5f Compare November 1, 2023 23:18
@leMaik
Copy link
Member

leMaik commented Nov 1, 2023

Rebased against latest master. Mapping for copper bulbs would be great. 💡

@leMaik
Copy link
Member

leMaik commented Mar 19, 2024

@JustinTimeCuber Given that #1621 is merged now, is this one ready for merge?

@JustinTimeCuber JustinTimeCuber marked this pull request as ready for review March 21, 2024 00:01
@JustinTimeCuber
Copy link
Contributor Author

I think so, as long as there isn't another new light source I haven't heard about

Comment on lines +536 to +540
Vector3 emittance = new Vector3();
doEmittanceMapping(emittance, emitterRay.color, scene, emitterRay.getCurrentMaterial());
emittance.scale(e);

result.scaleAdd(e, emitterRay.color);
result.add(new Vector4(emittance, 0));
Copy link
Member

Choose a reason for hiding this comment

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

Can we do this without allocating Vector3 and Vector4?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the Vector3 emiitance is definitely useful because otherwise we'd need a doEmittanceMappingR etc. but the Vector4 could be removed

Comment on lines +118 to +123
// fancierTranslucency.selectedProperty()
// .addListener((observable, oldValue, newValue) -> {
// scene.setFancierTranslucency(newValue);
// transmissivityCap.setVisible(newValue);
// transmissivityCap.setManaged(newValue);
// });
Copy link
Member

Choose a reason for hiding this comment

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

This seems unrelated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huh I wonder how that got there, I'll remove it

* (x, y, z): The color to use for the REFERENCE_COLORS emitter mapping type.
* w: The range surrounding the specified color to apply full brightness.
*/
public ArrayList<Vector4> emitterMappingReferenceColors = new ArrayList<>();
Copy link
Member

Choose a reason for hiding this comment

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

We allocate lots of materials (one per block type, one per triangle in the bvh). Will this become an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I could set it to null and only initialize when needed?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good

Copy link
Member

Choose a reason for hiding this comment

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

Also, maybe set it to null again when it's not needed anymore

Copy link
Member

@leMaik leMaik left a comment

Choose a reason for hiding this comment

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

Looks good, there's some debug code left and we should make sure that we don't increase memory usage too much (especially when color mapping isn't used).

Co-authored-by: Maik Marschner <[email protected]>
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.

3 participants