-
Notifications
You must be signed in to change notification settings - Fork 237
Updating the renderer to 1.21.10 #683
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
base: dev
Are you sure you want to change the base?
Conversation
…cture, including a constructor, a builder class, and placeholder methods. This completes the first step of the plan. I have created the three placeholder shader files: `raygen.rgen`, `miss.rmiss`, and `closesthit.rchit`, and placed them in the correct directory. This completes the second step of the plan. I have modified the `SPIRVUtils.java` file to include the new ray tracing shader types in the `ShaderKind` enum. This completes the third step of the plan. I have updated the `PipelineManager` to handle the new `RayTracingPipeline`, including adding a field for it, a method to create it, and updating the cleanup method. This completes the fourth step of the plan. This involved adding the `doRayTracing` and `bindRayTracingPipeline` methods to `Renderer.java` and calling `doRayTracing` from the `endFrame` method. This completes the fifth step of the plan. I have added the `enableRayTracing` option to the `Config` class and used it to conditionally execute the ray tracing code in `Renderer.java`. This completes the sixth step of the plan.
This commit resolves several compilation errors that were causing the Gradle build to fail. The following changes were made: - Removed a duplicated package declaration in `src/main/java/net/vulkanmod/vulkan/shader/RayTracingPipeline.java`. - Corrected the import path for the `Buffer` class. - Added missing static imports for Vulkan API symbols from VK11, KHRBufferDeviceAddress, and VK12. - Updated the cleanup method call from `free()` to `scheduleFree()` to match the `Buffer` class API. - Declared the missing `sbtStride` field.
Fix Gradle build errors in RayTracingPipeline.java
Saving the current state of the rendering refactor due to persistent tool service issues. This commit is not expected to build successfully but captures the changes made so far.
WIP: Save rendering refactor state
…ation Fix ray tracing shader binding table setup
…-dependencies Update Fabric dependencies for Minecraft 1.21.10
…-pipeline Update renderer integration for new pipeline API
|
It is still WIP, so I published it as a draft PR for now. If anyone has any suggestions or better yet, code, feel free to contribute. |
…er clarity and consistency.
|
Why do you update Java to 25 and lwjgl to 3.4.0, when minecraft only supports Java 21 and lwjgl 3.3.3? Especially the lwjgl change will cause minecraft to not start at all, as version 3.3.3 is a hard dependency. |
|
Focus only on what the PR goal is and don't make unnecessary/broken changes like these. You can make a separate PR for these. |
|
You also modify the config and options for no reason. Again, make a separate PR for these changes. |
|
You do a bunch of useless formatting changes in the shaders. Put these in a separate PR and stick to the 'old' formatting for now. |
|
|
||
| private static boolean doUpload = true; | ||
| private static final Set<VulkanImage> transitionedLayouts = new HashSet<>(); | ||
| private static boolean doUpload = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm talking about stuff like this.
|
I aligned the LWJGL version with MC to prevent classpath conflicts as @NXTler suggested. Let's see how it works out. |
I need Java 25 to minimize unsafe memory access, which could cause silent crashes and is also deprecated. Also, using the most recent Java LTS version is standard practice. I also attempted to use a newer LWJGL version because of this, but it seems like that's not possible. |
|
Forcing Java 25 on users to play a Java 21 game isn't the solution and unsafe memory accesses shouldn't happen in the first place. This is a completely unnecessary change, as you can see in the recent test builds. |
|
You also started adding new features again, put this in different PRs. |
LWJGL uses sun.misc.Unsafe internally in its MemoryStack and MemoryUtil classes, and VUtil also used it until I changed it to use FFM, so it did happen and I fixed it (well, as much as I could without entirely rewriting LWJGL's memory access methods). And the specific feature in question is necessary to complete the renderer refactor and make it work on 1.21.10. |
as far as i know about github , i dont think you can really make 2 pr's from one account due to the limit of 1 fork |
|
You can have different branches in a fork for different PRs with no issue. He should only focus on doing one thing at a time or his PR will never get merged. |
its choice of whoever has ability / perm to merge |
Do you have prove that it ever caused any issues, or did the AI tell you that? The official test builds for VulkanMod 1.21.10 work completely fine with java 21, so what's the point in fixing a not existing issue? Again, this is just forcing the user to use another Java version than the game they are playing. |
"The official test builds for VulkanMod 1.21.10 work completely fine with java 21" are you speaking of test build of this fork but with properties changed to use java 21 instead of this persons java 25 decision? |
|
Not at all, Collateral has been working on 1.21.10 for some time now, and we have test builds on the Discord. They have nothing to do with this fork in any way and are functional without Java 25. |
Can't Collateral commit those changes to the dev branch if they're mostly functional? I could build on top of his work instead of diverging in ways not intended for the project. |
i dont spend time on that platform therefore i didnt know, i may join. |
This PR updates the mod and refactors the renderer for 1.21.10 compatibility and prepares it for future enhancements like RT. It also includes changes to the README, refactors the mod to use Mojang mappings (due to better rendering class mappings) and adds other stuff.