#Blaze3d Extensions (1.21.6+)

1 messages · Page 1 of 1 (latest)

vale otter
#

Blaze3d is an incomplete rendering API, lets fix that

#

I've currently prototyped a few different things:

  • Adding copyBufferToBuffer to CommandEncoder for device-side buffer copies
  • A more complete version of the TextureView API currently part of B3D. GL 4.3 is required for full functionality.
  • A more complete set of draw commands. GL 4.6, 4.3, 4.2, 4.0, or CPU side emulation needed for some features
  • Allowing a pipeline to define multiple vertex input buffers, and per-instance input rate for a given buffer
autumn zephyr
#

When you say GL xyz is required, this would still work if the context is 3.2 with forward compatibility and you'd need to check for extensions, yes?

#

I am not even sure you need a brainstorming thread for these 😅

vale otter
#

Some things can be faked though.
Ie: a multidraw indirect call can be faked on a Mac with gl 4.1, because it supports indirect. We may still be able to have the APIs added advertise a capability it doesn't fully have. gl_DrawID can also be faked with uniform calls when doing that. not fast, but will work if you want to rely on gl_DrawID.

autumn zephyr
#

well i dont mean fake in particular, I am just hung up on the terminology 😄

#

this can use an extension that is core in 4.3, I assume?

#

meaning: we don't have to increase the minimum context version

#

but what we do have to supply most likely is a feature check on the APIs that do require more than 3.2 core?

#

(Unless mojang bumps their requirement to 4.1 as well)

vale otter
autumn zephyr
#

Yeah I see, design-wise we have to see how that works out. I'd prefer those "supportsXYZ" checks next to the methods we provide but if a feature spans multiple methods that can get tedious

#

pointing to the method that has to be used to check support for a featur before using the method is fine too ig

vale otter
#

With VK it's all available, GL would need to set based on what extensions it has.
Some things would need to throw errors if the capability isn't present.

autumn zephyr
#

yes that is fine if we provide proper check support for it, in my opinion

#

"proper". I mean easier to use than manually figuring out which GL caps are needed and checking LWJGL for it

vale otter
#

That's basically the intention.

autumn zephyr
#

I think we have the same idea anyway, just slightly different locations where to put those methods, etc.

vale otter
#

That is part of the brainstorming I want. I imagine API details are going to be the main thing discussed in this thread

vale otter
# vale otter I've currently prototyped a few different things: - Adding `copyBufferToBuffer`...

to be specific on the requirements
ref: https://www.khronos.org/opengl/wiki/History_of_OpenGL
GL_ARB_texture_view was made part of GL 4.3. On lower versions without the extension we will need to put in place restrictions on how the API gets used, as it must be faked.
GL_ARB_draw_indirect was made part of 4.0. Same idea, version or extension needed for making a draw*indirect command.
GL_ARB_base_instance is part of 4.2, but can be faked.
4.3 added GL_ARB_mulit_draw_indirerct, can be faked with draw_indirect.
and 4.6 GL_ARB_shader_draw_parameters, though this can also be (somewhat) faked, its also available in most cases on earlier versions.

#

GL_ARB_instanced_arrays is also 3.3, not 3.2, and is required for per-instance input rates. In practice it is always available.

keen pine
#

Compute shaders are a 4.2 thingy right?

vale otter
#

4.3

#

and im very not sure what adding those is going to look like

keen pine
#

No don't bother with that

#

Was just thinking

#

And they are like miles of for mac anyway

#

So forget that I asked

vale otter
#

"miles" VK says hi

#

also, i do want to add compute shader support

keen pine
autumn zephyr
#

yup

#

technically this can be feature-by-feature with maybe some foundational stuff in the first one. or, I am not yet grasping the scope

keen pine
#

Especially the stuff for which a 99% ARB exist

#

So it is basically put that all in

autumn zephyr
#

yeah i am not dead set on either

vale otter
keen pine
#

Then there are the features which require work arounds on older hardware / mac

autumn zephyr
#

just judge it based on line count

keen pine
#

Those that are trivial can go in with the basic infra

#

And then you go on a feature by feature basis

vale otter
#

initially, i want to get some of the more basic features (ie: buffer copy, draw commands) in, then later stuff like compute shaders can be looked at

vale otter
#

@exotic pumice @trail hearth ima bring you two in here for input becaue Flywheel

trail hearth
#

I have not thought about how a port to 1.21.6 would look like besides leaving the GL calls the same. Only backends issue GL calls, so abstracting the draw calls themselves as well doesn't seem like it would have too much benefit for Flywheel. It's unclear whether existing backends will actually work with Vulkan even if they use abstracted graphics calls, and the plan for that was always a dedicated Vulkan backend, which would likely be able to achieve higher performance anyway.

vale otter
#

i believe as it stands currently they wont, at a minimum because some free uniforms are used

trail hearth
#

Yeah

#

The backend compatibility checks also use GLCapabilities directly

vale otter
#

that would be easy to change

late lotus
#

I would make sure that each PR is digestible

#

Most importantly the PR description should make clear what the use case is, IMO. And it would be good to add some basic (!) tests in the process to help porting this stuff in the future, if possible (depending on the specific feature this may or may not be realistic)

exotic pumice
#

do yall have client side rendering tests working in ci? blurryeyeswheel

#

spinning up another backend (or butchering instancing) to target blaze3d is not unrealistic so long as we can provide runtime-generated shader source

vale otter
#

targeting B3D or "Neo3D" is probably the easier option for you to do, vs a jump straight to a VK backend

exotic pumice
#

oh definitely

vale otter
#

shader sources in B3D are provided through this

    CompiledRenderPipeline precompilePipeline(RenderPipeline var1, @Nullable BiFunction<ResourceLocation, ShaderType, String> var2);
#

you can provide whatever BiFunction you want there for getting the source

exotic pumice
#

and I have a feeling that a vk backend would need to target a specific vk mod (cough cinnabar) to truly be effective

exotic pumice
vale otter
#

only catch to be careful of is that it may not be called, the device is required to maintain an internal cache

vale otter
#

B3D, C3D, ill see myself out now

#

but in actuality idk what would be a good way to differentiate between Vanilla B3D vs Neo B3D

vale otter
exotic pumice
#

just a “use this source string please” would be amazing

vale otter
#

ie: support for a pre-recorded commandencoder/renderpass, without having to deal with VK itself directly

vale otter
exotic pumice
#

something like that yes

#

how much do b/c3d rely on the bifunction?

vale otter
#

but also, the way the cache(s) work,
sources is keyed off the resource location and type (in cinnabar), vanilla caches post-defines so the defines are used too
compiled is keyed off the pipeline by reference
so long as a given resource location and type always has the same source, the cache can be forgotten about

vale otter
#

the cache is required because of startup

#

theoretically, it could be disabled for when a bifunction is povided

#

so, providing one is asking for compile, or recompile, and (re-)fetch this shader's source

vale otter
#

vanilla behaves basically identically

vale otter
#

@fossil isle @late lotus @autumn zephyr

I have a question prompted by @fossil isle specifically mentioning my addition of R4G4_UNORM_PACK8 to TextureFormat given OpenGL can't support it at all (and also task/mesh shaders), should extensions to B3D in Neo be limited to what OpenGL can implement and/or what OpenGL's requirements are?

This can be broken down into a few different questions

q1: should things like R4G4_UNORM_PACK8 be included?
This specific instance doesn't really matter much (a shader can do the unpacking), but the idea of a definition for something OpenGL has no concept of. Obviously this means that Neo wouldn't support it, it would be expected that an alternate backend (ie: Cinnabar) could support it.

q2: should things that require not widely available extensions be defined?
Task/Mesh shaders are the best current example of this, where GL only has the GL_NV_mesh_shaders extension to support it. Similarly here, Neo would not need to support these, with the definition existing for alternate backends to implement.

q3: should the design of interfaces be restricted to what OpenGL requires?
The easiest example of this I have at the moment is glMemoryBarrier vs vkCmdMemoryBarrier. OpenGL only requires the second sync scope (see vk spec for what exactly that means) where VK requires both the first and second sync scope, as well as access. GL also only requires what is effectively VK's stage, while VK also requires the access be specified.

q4: should the design of interfaces allow/encourage usage that may have to or will always have to be decomposed for OpenGL?
glCopyBufferSubData vs vkCmdCopyBuffer2 comes to mind here. The former specifies a single range from a single source to a single destination. The latter allows for multiple ranges from multiple sources to multiple destinations. The latter can be faked by an OpenGL backend, but isn't necessarily the optimal API usage for GL.

#

Obviously, I am biased toward things that make it easier for VK, though I also realize its not reasonable to use a purely VK based design for a game written for GL

#

My opinions are
q1: Only if trivial. ie: TextureFormat enum entry sure, async operations no.
q2: Only if trivial. Shader type definition sure, pre-recorded commands (GL_NV_command_list) no.
q3: It depends on the different options, and would need to be considered on a case by case basis.
q4: Generally yes. In some cases doing so may help OpenGL drivers as well, though that isn't a guarantee.

barren citrus
#

wrt q1, can the value be gracefully downgraded to a known value if the backend doesn't support the value?

vale otter
#

if it could, that would fall under Q2 or Q4, so, no.

#

using R4G4_UNORM_PACK8 specifically as an example, the only way to emulate this on OpenGL is using R8_UINT and then unpacking in the shader with uvec2((packed >> 4) & 0xF, packed & 0xF). Theoretically a backend could do this, but doing the proper replacement in arbitrary shader code is not a reasonable thing to do.

#

it being so easy to suggest an alternative for that is why the specific instance is meh either way. But if something did exist that cant be so easily (by the mod author) 1:1 replaced, it would matter more.

barren citrus
#

yeah, my gut feeling says something that doesn't exist in opengl doesn't belong in the default impl

#

but rather than emulating with a R8

#

my consideration would be more, R8G8_UNORM or whatever opengl happens to support

#

something that is not as ideal but still works

vale otter
vale otter
barren citrus
#

oh this is pixel format? nevermind then

#

my brain was picturing vertex attributes / shader parameters

vale otter
austere wren
#

I think the impl definitely needs to throw a clear exception for Q1/Q2 if the answer is yes, so that it's easy to pinpoint usages of unsupported options

vale otter
#

Yes, though that would depend on where. GlConst.toGl*(VertexFormat) probably shouldn't throw , but GlDevice.createTexture should throw.

#

same idea for not having GlConst.toGl(ShaderType) throw.

#

doing that is also necessary regardless, as some things aren't always supported, but absolutely should be included (compute shaders, stb, ssbo, indirect), or could be included and are supported directly (texture formats S8, R5G6B5, R5G5B5A1, R4G4B4A4)

late lotus
#

What I would like to see (and it would help answer these questions I think) is why some features/extensions are useful

#

Blaze3D doesn't have some stuff that wouldn't be useful in the context of vanilla. So the question is why is it useful in the context of mods

#

We can easily make incremental improvements to the API as more use cases are discovered, but I am quite hesitant with a blanket "add as much stuff as I could think about" PR

#

Ah you did reply with a justification on the PR, which I haven't read yet

#

Might read it tomorrow

vale otter
#

TLDR of that: some are targeted at specific things (SSBO), and the less targeted ones are just adding constants that get passed through to keep people from trying to go around.

late lotus
#

I'll have a deeper look tomorrow. Just adding the other constants from the opengl spec is fine. Some things I'll have to investigate a bit more deeply

vale otter
#

TextureFormat: This is primarily targeted at data texture/UTB uses. Quartz has used textures of a particular format to store lighting data. Vanilla does to store cloud data. Flywheel does to store instance data. Each has their own specific needs and with that their own best texture format(s).

LogicOp/SourceFactor/DestFactor/PolygonMode/DepthTestFunction/AddressMode, these are all adding GL constants.

Storage Images (usage bit on textures) can be written to

for the GpuBuffer bits
Transform feedback: used by Quartz to support Iris shaders. Can also be utilized to fake compute on otherwise unsupported devices (Quartz also did this).
STBs can also be written to
SSBOs can be written to and also size limit of UBOs removed (and can be arrayed with undefined size). Cinnabar uses these behind the scenes for world rendering.
Indirect, shouldn't need much explanation. Sodium, Cinnabar, Quartz, Flywheel, and probably more all use this.

The last two bits are for performance. Cinnabar at least will use them, and its extremely useful to have "Vanilla" use them too.
FRAME_TRANSIENT is targeted at things like per-frame uniform buffers. Think projection matrices, or other per-frame state. Anything that gets updated every frame. For Cinnabar, this allows me to automatically use multiple buffers to back it, that are also extremely quickly allocated.
ONE_TIME_USE is the above, but for things that get used exactly once. This is targeted at things like rendertype immediate draw buffers. For Cinnabar, this allows me not copy to GPU local memory (having it read directly from the CPU memory instead).

#

OpenGL can also utilize particularly FRAME_TRANSIENT to similar benefit

vale otter
#

The last one is ShaderType. What I said on github is basically it, but to be more specific.

Compute I want to add support for to Neo, multiple mods use this currently (again going to point at Flywheel and Quartz).
Task/Mesh shaders I plan to support in Cinnabar, but because OpenGL has them too putting them here also leaves open the option of some other GL based backend (including, extension/mixins) adding support via GL_NV_mesh_shader.
Tess and geometry are unlikely to get used, though theoretically very cool, and could probably be removed. They were added mostly for completeness.

vale otter
# late lotus We can easily make incremental improvements to the API as more use cases are dis...

incremental improvements
notable thing to consider, any additions to the API definition that aren't themselves (default) implemented within the API are a breaking change. while adding every feature isn't a good idea, parts of a feature should probably not be left out simply because there isn't a specific articulable today use for that subset of it.

ie: if a draw command is added, such as an instanced "drawArrays" command, i would argue that all reasonable draw command variants should be defined, even if not implemented immediately. If applicable, an implementation could be added to Neo later without being a breaking change.

late lotus
#

OK, got started on the PR at least

#

DepthTestFunction, DestFactor, LogicOp, PolygonMode, SourceFactor, AddressMode extensions are certainly fine (especially for destfactor and sourcefactor I wonder why mojang didn't just go for all of them)

#

didn't have time to look at the rest yet - tomorrow? 😇

#

what is the minimum GL version that MC supports? 3.3?

fossil isle
#

3.2

late lotus
#

ah

#

then GL33C.GL_SRC1_COLOR seems a bit problematic?

fossil isle
#

PolygonMode seems odd to me as wireframe is already a rare thing to use (it's basically just used in debugging IIRC) but a single value doesn't hurt so whatever. For the rest I agree, though for LogicOp I'm curious what one would use that for in general.
What I'm concerned about is TextureFormat. Adding all those variants feels like insane bloat to me and unnecessarily confusing for less rendering-savy people.

late lotus
#

I would agree for TextureFormat, but haven't had the chance to look at the details yet

#

in general I am also very wary of adding anything that requires checking for extensions

#

anything going beyond GL 3.2 should probably come with solid justification

trail hearth
#

Why are some vanilla enum fields reordered?

late lotus
#

I suppose that rogue just copied all GL fields from the spec, and forgot to reorder some of them?

vale otter
#

technically copied from VK spec, but GL has the same ordering iirc

vale otter
vale otter
# fossil isle `PolygonMode` seems odd to me as wireframe is already a rare thing to use (it's ...

What I'm concerned about is TextureFormat. Adding all those variants feels like insane bloat to me and unnecessarily confusing for less rendering-savy people.
What ones would you keep then? The vanilla set alone is not enough to cover modded uses (I was running into exactly that problem).
The largest block of them (33 additions, 3 vanilla already had) is the combination of 8, 16, or 32 bits per element, 1, 2, or 4 elements, and then UNORM, SNORM, UINT, SINT, and SFLOAT data type
The packed ones could be done in shader with R16_UINT or R32_UINT, except 999e5, but are nice to have. They are reasonable to remove if a smaller set is desired.

fossil isle
#

Ones I would definitely remove are the compressed ones, ones GL does not support (this whole thing should still be GL-first since that's what Minecraft is natively built on), stencil-only ones and the packed ones. A smaller set is IMO definitely preferable to retain people's sanity.

autumn zephyr
#

I use it to draw an XOR inverted overlay over some UI

vale otter
# fossil isle Ones I would definitely remove are the compressed ones, ones GL does not support...

non-GL and stencil-only fair enough.
packed i would prefer to keep, as they can be a sanity saver for those using them. particularly for passing into an existing shader expecting a multi-channel texture.
compressed are a fairly use-case specific thing, its large textures where they are the most beneficial. I'd like to see them supported and used, but can also understand not wanting them there for a smaller format set instead.

#

with packed/compressed removed as well, there would be 41 total formats.

8bit R, RG, RGBA : UNORM, SNORM, UINT, SINT
16bit R, RG, RGBA : UNORM, SNORM, UINT, SINT, SFLOAT
32bit R, RG, RGBA : UINT, SINT, SFLOAT

D16_UNORM
D24_UNORM
D32_SFLOAT
D24_UNORM_S8_UINT
D32_SFLOAT_S8_UINT

vale otter
#

@fossil isle what would you want the use comments to be exactly?
with USAGE_TRANSFORM_FEEDBACK as an example. That's saying "this buffer can be used as the capture destination for transform feedback operations".
To me, something explaining it to that level seems redundant, but explaining further such that its not redundant also seems out of scope of a comment on a usage bit.
the others would also be very similar.
even the VK spec basically just repeats what the enum name is, but gives some/all calls where it can be used

VK_BUFFER_USAGE_TRANSFORM_FEEDBACK_BUFFER_BIT_EXT specifies that the buffer is suitable for using for binding as a transform feedback buffer with vkCmdBindTransformFeedbackBuffersEXT.
fossil isle
#

The way the VK spec describes it seems decent to me. You need to keep in mind that a lot of people who are not nearly as well versed in all this as you are will be looking at this.

fossil isle
vale otter
fossil isle
#

Definitely

vale otter
#

instead or additional?

fossil isle
#

Additionally

vale otter
#

aight

#

following what VK does for wording, i think it would be good to list where it gets used, but in all cases where they get used is yet to be implemented, would it be fine to leave a {NOT_YET_IMPLEMENTED} placeholder?
ie:

USAGE_TRANSFORM_FEEDBACK specifies that the buffer is suitable for using for binding as a transform feedback buffer with {NOT_YET_IMPLEMENTED}.
See OpenGL wiki for details on what this feature is: https://www.khronos.org/opengl/wiki/Transform_Feedback
#

that would later get replaced with a link to something like RenderPass.bindTransforrmFeedbackBuffers, though that doesnt exist (yet)

fossil isle
#

Sure, that sounds reasonable to me

vale otter
#

the client config's reasoning for D24S8 being an option "slightly reduce vram", for which the compressed textures would make a far larger difference.

#

that would reduce it to
color variants (36 total), D32F, D32F_S8, for 38 options

#

i don't think the color variants should be reduced further.

fossil isle
vale otter
#

as-is, color is already reduced to one option per bit width, component count, and component data type.
the only three that i find even remotely reasonable to remove are the 16_SFLOAT ones, as java does not have native support for fp16 conversion, though it can be done (or they can be used directly on GPU)

#

these three, specifically

R16_SFLOAT
R16G16_SFLOAT
R16G16B16A16_SFLOAT
#

additionally removing those would bring it down to 35 total formats

fossil isle
#

Hmm, 1.21.7-rc1 deprecates RenderPipeline.Builder#withColorLogic() and replaces the only use of LogicOp - color inversion for text selections - with a dedicated BlendFunction

late lotus
#

shartte had a use case for logic op

fossil isle
#

But which specific one? If he just uses the existing LogicOp.OR_REVERSE, then he could almost certainly just switch the same way

vale otter
#

XOR_INVERTED

vale otter
frank sedge
vale otter
#

XOR_INVERTED actually isn't a thing, nor is XNOR, @autumn zephyr what logic op did you use exactly?

autumn zephyr
#

XOR IIRC

#

if they do blend function for inversion i can likely use that

#

I use

        GL20.glLogicOp(GL20.GL_XOR);
        GL20.glEnable(GL20.GL_COLOR_LOGIC_OP);
vale otter
# fossil isle Hmm, 1.21.7-rc1 deprecates `RenderPipeline.Builder#withColorLogic()` and replace...

notable thing, this isn't a perfect 1:1 replacement, the OR_INVERSE did behave differently
the logic op definition is a bitwise s ∨ ¬ d, or in java bitwise ops s | ~d
the new op is (1.0 - s) + (1.0 - d), and because unorm format 1.0 - x is equivalent to ~x, so they have effectively ~s + ~d (note: thats a normal add, w/ carry), may give a similar looking result but its not the same thing

#

actually, wait, im slightly wrong
they are using s * (1.0 - d) + d * (1.0 - s)

#

still a different operation, that gives a good enough result, but not the same result

vale otter
#

oh, fun, so, potentially why they dropped support for it is because Metal (and by extension MVK) doesn't have LogicOp support

#

Apple uses undocumented metal features to implement OpenGL, so obviously the hardware can do it somehow

#

but, if its deprecated then intent does seem to be to remove the feature, and ^ that is probably why

barren citrus
#

rip

#

wait does that mean they are planning to make the backend Vulkan? thonk

#

or just expecting opengl to become crippled in the near future?

mighty beacon
#

Probably leaving the first door open

#

Since it doesn't really affect them anyway

vale otter
#

Gegy mentioned this is actually because of Qualcomm's OpenGL driver

#

Apparently it throws a fit if you try and use logic op via OpenGL (it's fine via VK)

vale otter
autumn zephyr
#

like. what

hushed stump
#

behold more oppengl cursed driver crap

vale otter
#

No idea, ask gegy

hushed stump
#

i assume its windows? again?

autumn zephyr
#

I'll just accept it

vale otter
#

Windows aarch64

hushed stump
#

becuase, i again, dont see mesa having this kind of shit

autumn zephyr
#

Yeah the new type of Windows ARM laptops, most likely

#

Well. "new"

#

You know what I mean

hushed stump
#

its always windows, unless its mac

vale otter
#

I'll have you know there is a desktop one

autumn zephyr
#

I got so burned by broken AMD OpenGL drivers a decade ago I'll never forget it 😄

vale otter
#

Assuming you ordered it at the right time, and didn't get your order canned before it shipped.

#

mine shipped first

autumn zephyr
#

I think I saw an ARM based Windows MiniPC, but could be hallucinating

vale otter
#

The devkit is the only one I'm aware of

#

And afaik, it's the only shipped example of the X1E-00-1DE variant

fossil siren
#

most rendering bugs afaik have been macs lately

#

this is a rarer occurrence of a windows issue

hushed stump
#

becuase mac gl is mega fucking cursed

autumn zephyr
vale otter
#

yuuuuuup

#

I still got mine though

#

It's damn fast, and idles at like 4w

autumn zephyr
#

I am kinda a fan of Windows supporting new architectures, tbh

vale otter
#

but, weak memory ordering

vale otter
#

logicop being broken on a supported platform (Qualcomm) though, maybe it would be better to actively discourage its use, or note a warning about that and add a capability check to the GpuDevice for it.

late lotus
#

@vale otter logicop has existed since OpenGL 1.1

#

how can it be broken

vale otter
#

dont ask me, ask Qualcomm

#

and its not a hardware issue, heres it working fine on VK

2025-06-25_23.04.12.webp
late lotus
#

but it's fucked via opengl on the same chip?

vale otter
#

yup

late lotus
vale otter
#

though they dont have a native OpenGL driver

#

afaik its GL-> D3D12

late lotus
#

ah, and maybe D3D12 doesn't support it?

#

or there's a bug in the driver

vale otter
#

for all intents and purpose, D3D12 and VK are the same thing

#

for a common feature (like logic op), id expect any bug to be present with both

#

and half the D3D12 driver is actually implemented by MS, not Qualcomm, so, almost certainly this is in the GL -> D3D12 layer

#

their buffer upload was also working incorrectly, so theres that too

late lotus
#

hmm

vale otter
#

that driver just has issues

late lotus
#

so we have two choices, either include extended logic ops or not

#

say I have a qualcomm chip... is there any workaround?

vale otter
#

w/ OpenGL and logic ops, afaik, no

late lotus
#

ok then my verdict is no logic op for now

vale otter
#

IMO, it should be either one way or the other entirely
option 1) feature is there, and we add the extensions for the entire feature
option 2) we dont add it, and if its left in by mojang make it very clear that it shouldn't be used (and why)

late lotus
#

it's still there in 1.21.7-rc1

vale otter
#

and rc2

#

its just marked deprecated

#

and not used by vanilla

late lotus
#

ermm yes

#

I think we can add a comment saying that it shouldn't be used

#

re DestFactor I would avoid adding anything that is not in GL 3.1

#

(that applies to the whole PR 😛 )

#

I'm gonna leave some comments on the PR so we don't forget

vale otter
#

also, 3.2*

late lotus
#

ermm yes 3.2

#

I would rather not make it easy for mods to accidentally use features that are not supported on all platforms

vale otter
#

because validation will be required regardless, we could validate use against 3.2 available things unless a specific i know what im doing, and that i need to check shit option is enabled. in IDE.

late lotus
#

well I assume that the people who want to go beyond 3.2 features also want to do it in prod, not just in their IDE

vale otter
#

right, in prod we'd default to hardware available or
that i know what im doing could actually be done on a per-feature basis (including at runtime), where the mod has to ask for something to be turned on/available, and if it can't be will get an error thrown at it by B3D.

VK does a similar thing when creating a device. you first must ask for its capabilities, then when creating it must tell it any optional features to be enabled.

#

and if you ask for something it doesnt have, it will fail to create the device

#

this couldn't be filtered on a per-mod basis, but would make sure that if a feature gets used in dev, they did ask for it, and therefore have somewhere specific that the error would get thrown if its not available (in prod)

late lotus
#

okay that seems reasonable to me; let's ask @autumn zephyr and @fossil isle for their opinion as well.

TLDR: the discussion is about optional GL features. Rogue proposes that any feature beyond GL 3.2 require explicit enabling by mods, and if the GPU does not support it they will get a crash during initialization

#

which means that you can't accidentally use optional features, and that if you try to use an optional feature on hardware that doesn't support it you will get a predictable runtime crash during game loading

vale otter
#

we could also do this for known problem features, like logic-op
and dev should default to only 3.2 available, even if your hardware has more, so you even then must explicitly enable a i know what im doing mode before it will even report that it has more than the bare minimum of features

late lotus
#

I would probably try to make dev and prod behave similarly if possible

vale otter
#

the dev mode thing is behave as if you have a 3.2-only card

late lotus
#

I don't see a need for a difference - we could simply make both dev and prod require the feature to be turned on?

vale otter
#

ehh, yea, i guess its probably fine to do that

#

i was thinking making the reporting side behave as if its 3.2 only by default in dev, even if you have a 4.6 card.
related thing that could be done is programmable simulated modes for specific for GL versions and/or extensions. so you could reasonably test what happens with the GL version/extensions that mac has, for example, without actually being on a mac.

late lotus
#

yeah that would be quite nice I suppose

#

so that you can try many combinations without having to manually enable/disable stuff on your end

vale otter
#

and i have most hardware, so i could actually build real feature set lists for most hardware. it wont be a perfect 1:1, but it would allow devs to catch most errors

#

glares angerly at shaders

austere wren
#

because what you're describing is only enforceable if (a) everyone uses B3D and as you mentioned (b) doesn't use extra features in shaders

fossil isle
#

Requiring explicit opt-in for features from newer GL versions sounds fine to me. We could potentially even re-use the main render target setup event for this.
As for dev, I wouldn't force the game into effectively 3.2 mode but having ways to simulate limited environments sounds like a good idea to me.

vale otter
austere wren
#

Ideally something like Mesa itself would provide the option but that will probably never happen

vale otter
#

IMO, if a mod is going around b3d, that's its own problem, and is entirely into you know what you are doing territory, as it is today.
for shaders, we could look at the #version declaration and search for extensions too, its not impossible, and that would probably cover most things for usages

#

its driver bugs that you cant replicated

#

for example
HD5000 (TS2) has issues with image store working properly, on linux
HD4000 (TS1) is liable to just crash with some shaders, but theres no way to know why exactly

#

or mac not implementing separate shader objects properly

#

theres no good way to check for those, and thats true today as well

#

any addition of validation is strictly better than what is available today, where you just hope for the best and that you didnt fuck anything up

autumn zephyr
autumn zephyr
#

the device would throw regardless of feature-test?

vale otter
#

something analogous to what VK does for device creation, not perfectly identical, but the general idea

autumn zephyr
#

i have no idea what VK does ofr device creation 😄

vale otter
#

ill explain, gib sec

autumn zephyr
#

I am not sure where Tech is going entirely, but if he's trying to avoid Mods reaching for some GL constants that aren't in the base set, there are a couple options, yeah

#

one is the mod having to "unlock" the extension via a static method call in their mod constructor, otherwise the device throws if you try to use it
in a modpack you'd end up with cross-contamination, but in your own dev-env likely not

vale otter
#

with VK, to use a feature, it must be explicitly asked for during device creation,
this is done via VkPhysicalDeviceFeatures* structs, where you set bunch of bools.
if a feature isn't enabled, it doesn't need to work, and validation errors with be thrown (if a validation layer is enabled).
you can (should) also ask the device for what it support beforehand, though technically you don't have to.

mighty beacon
#

That reminds me of a fun bug with LWJGL where trying to create a Vulkan context on recent graphic cards would explode due to not enough stack space

vale otter
#

as a very real example of this, logicOp is part of the base VK 1.0 VkPhysicalDeviceFeatures struct
a device may report false, in which case you must not enable it
if you ask for it, the device will throw an error
if you dont enable it, and try and use it anyway, a validation layer will catch that.

mighty beacon
#

Thankfully that is solved nowadays

vale otter
vale otter
vale otter
#

very minor point but technically it would be better if it could be done at that stage.

#

i think thats before mod constructors fire though, so, idk

vale otter
#

this also means all the "requires GL 4.X" comments could instead be targeted at the reported device capabilities, and then those can link to the GL/extension/VK/whatever requirements, and note issues with features, all in one central place

#

keeping that centralized should help with keeping it up to date as new driver issues are found too, because those will exist

autumn zephyr
#

Since we moved the constructor calls for client mods to before Minecraft is initialized (Unless I am just hallucinating us doing that)

#

It might actually be easier to just expose static methods for this, but we're free to also add an event

vale otter
#

cool, the minor point that lets me do is leave them disabled in Vulkan if not asked for

autumn zephyr
#

or many events, whatever suits us

#

well kinda, how does that interact with earlydisplay?

vale otter
#

early display has its own VK instance and device

autumn zephyr
#

for GL the device will already be enabled, theoretically you might have to just do a validation-layer style thing in our side of the device

vale otter
#

for GL, doesnt matter

#

in reality, id be implementing a "validation layer" device in Neo anyway, to support doing this

#

doing it in patches would suck, and that also will catch people trying to reach around and cast it to GlDevice or whatever

late lotus
#

I'd like an event rather than a static method call

vale otter
#

they'd be forced to use reflection (or turn validation off), which ofc is possible, but if they are going that far im not going to try and stop them

autumn zephyr
#

yeah event is fine, now that we can do it

#

we can technically track which mods request which features that way

late lotus
vale otter
vale otter
#

so, you get an error of mod xyz requested graphics feature w that is not available

#

for all mods that did, and all the features they did too

autumn zephyr
#

do we know when the event is emitted which features are available? 🤔

#

well, technically you can emit it during the device creation I guess

vale otter
#

yea, during device init is probably when this should get fired

#

but i can also poll VK before that ezpz

#

and GL, context exists the whole time, so, also just a matter of polling it whenever

autumn zephyr
#

That would actually give mods a chance to also efficiently feature test in the event and configure themselves accordingly

vale otter
#

mhmm

#

before basically any other init too

vale otter
#

other fun thing this allows, almost any API addition could be done without it being an IMPL breaking change now.
to enable something, the impl must report it as available, and if you use it anyway that's bad on the using mod.
that makes a default to throw InvalidUse or similar actually correct, because that backend doesnt support the feature

#

new enum values or bitfields are also similarly a non-issue, because its invalid on the using mod side to enable/use them if the backend doesn't report support

autumn zephyr
#

well careful if you add anything to enums

#

you can run into switch expression issues

vale otter
#

JVM will throw an error on that

autumn zephyr
#

yes, exactly!

#

😄

vale otter
#

and/or default throw InvalidUse

#

again, this is invalid on the using mod side

autumn zephyr
#

no, a mod could just as well do switch on the same enum type

#

nothing forbids that

#

case for all existing issues, and receive a new enum value from another mod

#

you add that -> boom

#

adding enum values is likely still a break from our end

#

needs more discussion I suppose

#

since technically having runtime extensible enums lead to that problem too

vale otter
#

add SSBO: older backend doesnt report support for SSBO (because it didnt exist), you cant enable SSBO, passing SSBO should explode regardless

autumn zephyr
#

you're thinking about this wrong

vale otter
#

oh, does the JVM throw on classload?

autumn zephyr
#

i am talking about a mod doing:

switch (someOtherModsTexture.format) {
... list all cases of vanilla ...
}

#

if we add to textureformat enum, the mode consuming here did nothing wrong

#

but will still throw if another mod also didn't do anything wrong (by requesting use of the new textureformat enum value and using it)

vale otter
#

oic, yea, yea that could be an issue

autumn zephyr
#

but, as I said, we kinda already declare enums as extensible

vale otter
#

it still reduces what is a breaking change at least

autumn zephyr
#

which leads to this problem randomly at runtime by other mods adding enum values

#

it needs some discussion

#

since we likely dont want texture formats to be runtime extensible

vale otter
#

can add indirect commands, default throw them, and then backends can implement when they feel like it

vale otter
autumn zephyr
#

but we might make it an explicit properties of those rendersystem enums that they're intended to be extensible going forward

#

but... no idea how to formalize that I think we have not had that case yet

vale otter
#

but its stuff like the dual source blending where its behind GL 3.3

#

or compute shaders behind GL 4.3

#

switches in other mods exploding is the main issue, the backend exploding isn't an issue anymore though

vale otter
autumn zephyr
#

if you tell mods that they have to expect values to be added to these enums (in whatever way we chose) also applies to backends, so they'd also be "safe" against added stuff

#

and yeah if everything added (that we know is not OGL 3.2 for example) is feature tested, that makes it relatively safe to add new stuff i think

#

*add new stuff later as a non breaking change

vale otter
#

GL 3.2 available stuff could be added as non-breaking with it behind a feature test until the next BC window where its then unlocked always

#

so, adding new stuff could be made "always safe" with that

vale otter
#

PR opened for device configuration, https://github.com/neoforged/NeoForge/pull/2399
with that, most, but not all, additions could be done as non-breaking. Some very heavy validation could also be done.
main open question i have is how to communicate to mods that the enums in B3D may get extended without being considered a breaking change, and that they need to accoutn for that

late lotus
#

We could maybe annotate some enums

#

That would just be a documentation annotation

vale otter
#

i have no qualms with that, but is it obvious enough?

hushed stump
#

I can't think of a way to communicate that, besides switching the enums to interfaces with opaque impls, but that destroys switches

keen pine
#

This looks very nice

#

Yeah other then interface with static values

#

I see no other option

vale otter
keen pine
#

Switch will always barf on runtime extensions

vale otter
#

but, also, they are enums from vanilla, so, kinda a problem to change them from that

hushed stump
keen pine
vale otter
#

the extensions would be done at compile time, but potentially after the mod is compiled

hushed stump
#

Just make them as IExtendableEnum or w/e

#

There's nothing more we can realistically do without destroying enum switch

keen pine
#

Cause it has the exact same effect from a modders perspective

vale otter
#

Yea, it is the same effect

#

Marking them as "runtime extendable" the same with others are is probably the right option then

late lotus
#

No, don't mark them as runtime extensible

#

We don't want mods to extend them

vale otter
#

I mean, it's not really invalid for a backend to extend them at runtime

late lotus
#

It's invalid for most mods to do so

#

It does not fit the semantics of a runtime extensible enum

#

It's a bit of a shame that Java doesn't have a way to mark enums as non-exhaustive

vale otter
#

glares at c++ where it's just an int, and you can cast it

late lotus
#

The non_exhaustive attribute is good shit 😄

vale otter
#

time to rewrite MC in Rust?

late lotus
vale otter
#

I do like myself some unsafe behavior through, and rust doesn't like that

late lotus
#

You can think of Valence as a game engine for Minecraft servers.

#

This is crazy

late lotus
vale otter
#

The borrow checker would hate a lot of my concurrency code. It's heavily reliant on some pretty specific visibility ordering behavior to work right. bunch of manual memory/compiler barriers to get the compiler to do the right thing

late lotus
#

unsafe {} is fine to use in targeted places

hushed stump
#

Just document it then, nothing else we can do

late lotus
#

I think an annotation or interface is the right thing to do. Probably annotation

#

To make sure that even people who don't have sources attached see it clearly

hushed stump
#

Sure, INonExhaustiveEnum or something

vale otter
#

where should that interface go?

#

IExtensibleEnum is in FML, but that doesn't seem like the right spot for this

fossil siren
#

I think an annotation makes more sense as it's metadata

vale otter
#

same question on where it goes, and thats just a difference of @interface vs interface

fossil siren
#

and somewhere in an internal neo package

vale otter
#

@NonExhaustiveEnum added, and validation for the currently known enum values

#

also added validation for known usage bits

#

LogicOp was specifically not marked as NonExhaustive due to its issues

#

@late lotus @autumn zephyr @fossil isle 2399 should be in a good enough state for review now, if you want to actually look it over

late lotus
#

I want to have a look tonight

late lotus
#

ok I had a look and would recommend to focus on solving one problem at the time

vale otter
#

This is the "don't use features that aren't supported" and the extremely related "what are the usage limit" problems, almost entirely.

#

I could split it a bit, but a good chunk has to be done at once

late lotus
#

I don't see why it would need to be done at the same time

#

checking if the LogicOp is supported seems completely independent from querying the max texture size

vale otter
#

Strictly speaking, it doesn't have to be. If you'd like the PRs to be as granular as i can make them, I can do that. However, both of those things are also reliant on the boilerplate for the event. The validation layer is then also reliant on both of those, and would have to be created (or at minimum updated) after all three+ (base event, logicop enforcement, texture size query, etc) previous PRs. I can split it into many PRs like that if you would prefer.

vale otter
#

@late lotus for the features interface, like this better?

public interface GpuDeviceFeatures {
    /**
     * LogicOp is problematic on Qualcomm GPUs via OpenGL
     * LogicOp is unavailable on MacOS via Vulkan
     */
    boolean logicOp();

    void enableLogicOp();

    /**
     * Example for something that doesnt exist yet
     */
    default boolean someNewFeatureThatsUnsupportedCurrently() {
        return false;
    }

    default void enabledSomeNewFeatureThatsUnsupportedCurrently() {
        throw new UnsupportedOperationException("This really cool feature is unavailable");
    }
}

and an implementation of that interface

public class NeoGlDeviceFeatures implements GpuDeviceFeatures {
    private final NeoGlDeviceFeatures availableFeatures;
    private boolean locked = false;

    private boolean logicOp = false;

    public NeoGlDeviceFeatures(@Nullable NeoGlDeviceFeatures availableFeatures) {
        if (availableFeatures == null) {
            querySupport();
            this.availableFeatures = this;
        } else {
            this.availableFeatures = availableFeatures;
        }
    }

    private void querySupport() {
        lock();
        logicOp = !(Util.getPlatform() == Util.OS.WINDOWS && Util.isAarch64());
    }

    @ApiStatus.Internal
    public void lock() {
        locked = true;
    }

    void checkLocked() {
        if (locked) {
            throw new IllegalStateException("Cannot set features on locked feature set");
        }
    }

    @Override
    public boolean logicOp() {
        return logicOp;
    }

    @Override
    public void enableLogicOp() {
        checkLocked();
        if (!availableFeatures.logicOp()) {
            throw new UnsupportedOperationException("LogicOp is unavailable");
        }
        logicOp = true;
    }
}
#

what the event would look like

public class ConfigureGpuDeviceEvent extends Event implements IModBusEvent {
    private final GpuDeviceProperties deviceProperties;
    private final GpuDeviceFeatures availableFeatures;
    private final GpuDeviceFeatures enabledFeatures;

    public ConfigureGpuDeviceEvent(GpuDeviceProperties deviceProperties, GpuDeviceFeatures availableFeatures, GpuDeviceFeatures enabledFeatures) {
        this.deviceProperties = deviceProperties;
        this.availableFeatures = availableFeatures;
        this.enabledFeatures = enabledFeatures;
    }

    public GpuDeviceProperties getDeviceProperties() {
        return deviceProperties;
    }

    public GpuDeviceFeatures getAvailableFeatures() {
        return availableFeatures;
    }

    public GpuDeviceFeatures getEnabledFeatures() {
        return enabledFeatures;
    }
}

firing the event

        deviceProperties = GpuDeviceProperties.create(GpuDeviceProperties.class, propertiesMap);
        final var availableFeatures = new NeoGlDeviceFeatures(null);
        enabledFeatures = new NeoGlDeviceFeatures(availableFeatures); // memeber that can later be fetched from the GpuDevice
        ModLoader.postEvent(new ConfigureGpuDeviceEvent(properties(), availableFeatures, enabledFeatures));
        enabledFeatures.lock();

and then using it would be basically what you gave as an example for

if (avaialbleFeatures.logicOp()) {
    enabledFeatures.enableLogicOp();
}
#

a base class for backends to extend could be made, but likely wouldn't be verry helpful because in my case id probably be directly getting/setting these from/in the VkPhysicalDeviceProperties and friends structs, so it wouldn't be in java bools in the same way as GL would have

autumn zephyr
#

I barely had a chance to look at it yet but that class definitively needs a class-comment

#

pointing out what it's for, and potentially specifically call out the procedure for backend implementors

#

oh and a secondary note which is less concrete: Why did you call it neo3d?

#

As an extension of blaze3d, I'd just have put it in a package named as such

vale otter
#

separation between vanilla B3D and Neo B3D, though it could be renamed to whatever

autumn zephyr
#

you are already in a neoforge package...

drowsy aspen
#

Wildfire3d, after the blaze miniboss that lost a mobvote then was added to dungeons

autumn zephyr
#

i think any neo extensions to a vanilla system should be relatable to that vanilla system (so just name the subpackage in the neoforge structure blaze3d)

vale otter
#

I'll probably leave renaming that for after other issues get solved though, because that's just a file move

autumn zephyr
#

that's fine

vale otter
autumn zephyr
#

yes but without those docs, reviewing is also harder

#

because I have to actually reverse engineer what you wanna do first

vale otter
#

Fair

late lotus
#

the reflection stuff with the propertiesMap should definitely go as well

autumn zephyr
#

How is this mechanically going to work for a consumer that is aware of an implementations extension?

late lotus
#

what do you mean?

autumn zephyr
#

Yes please, no reflection magic

#

Well the class I saw seemed intended to be extended to sth like VkGpuProperties

#

With extra stuff on it

#

If that was not the intent I understand the design even less 😄

late lotus
#

maybe we should do the properties in a separate PR, and just focus on doing something reasonable with the features

autumn zephyr
#

Hmhmhmhm, there are actually a lot of designs possible I think that we've not yet explored for this, this is not particularly performance sensitive code, I think, so one object per property would also work (which could encapsulate the metadata you wanted to encode such as its default, and bounds)

#

But for that I think I'd need to know more about what the design drive behind the properties is 😅

vale otter
autumn zephyr
#

Hehehehe I love that they took notes from DirectX COM API versioning 😄

vale otter
#

It's also very needed for the validation layer to make sure people aren't doing invalid things

#

Ie: ubo size limit

#

Or alignment (Mojang screwed that up in a snapshot)

vale otter
wraith osprey
vale otter
wraith osprey
#

Huh. Interesting, I didn't know that existed

wraith osprey
#

Looking at it though, that seems to be a fairly recent extension, and we can't assume that devices will actually support it for... well, yes amount of time

vale otter
#

It's mostly just an informational extension, it's perf characteristics will predate it

vale otter
vale otter
vale otter
late lotus
autumn zephyr
#

ah @vale otter you didnt really make the PR to add render abstractions for earlydisplay yet, yeah?

#

we can do that later in a non-breaking way imho

vale otter
vale otter
late lotus
vale otter
fossil isle
vale otter
#

if its one impl, id want the mutable to be lockable

#

but 3 impl (two being API agnostic) is also an option, and honestly probably not that bad of one, very similar to what xfact suggested on github

mutable (can be its own class, no interface needed for the setters)
api-available (polls the underlying API for whats actually possible)
immutable (could be a record, created from either of the other two)

#

the API specific one wouldn't even need to have the fields itself

#

note that the immutable one being a record here works because it constructs itself from another interface instance, which i didn't think about doing previously

#

i imagine that works for the both of you?

#

properties could also be done in a similar way, where its a record constructed from an interface of basically itself

fossil isle
#

What do you mean by "record constructed from an interface of basically itself"?

vale otter
# 
interface GpuFeatures {
    boolean logicOp();
}
record EnabledGpuFeatures(boolean logicOp) implements GpuFeatures {
    EnabledGpuFeatures(GpuFeatures features) {
        this(features.logicOp());
    }
}
#

then add a mutable interface impl (doesn't need to be lockable), and api specific polling impl like your github comment has

fossil isle
#

Ah, ok, sounds reasonable then

vale otter
#

updated PR with basically that, for both features and properties

vale otter
keen pine
#

I opened that PR earlier today...

#

So unsure

#

Let me re-review it

fossil isle
#

I'm honestly still not a fan of making GlDevice abstract and extending it instead of either patching the custom stuff in or putting it in an extension interface that's interface-injected onto GlDevice. The specific provided example with glDrawElementsInstancedBaseVertexBaseInstanceARB() doesn't convince me either as I don't think the difference between ARBBaseInstance.glDrawElementsInstancedBaseVertexBaseInstanceARB() and org.lwjgl.opengl.ARBBaseInstance.glDrawElementsInstancedBaseVertexBaseInstanceARB() is relevant (I hate static imports with a passion, they just make it unnecessarily hard to find where a method or field came from [queue rant about C/C++ headers doing the same thing], which is why I won't even consider the option of omitting the class name)

vale otter
#

i have the exact opposite opinion, having the class name (and especially fully qualified) there adds unnecessary clutter that makes it harder to read, particularly when basically all documentation assumes a single namespace (because C)

#

if i need to find exactly where its from, my IDE can do that just fine

fossil isle
#

The single namespace thing is one of the details I despise about C++ in particular. The same namespace (or even none at all) is used across a stupid amount of headers and tells me exactly nothing about were a function comes from (granted, in C++ looking at the impl is often futile anyway, especially when it's about something in the stdlib).
The "my IDE can find it just fine" justification falls apart for the same reason why we forbid wildcard imports: I should be able to read and understand the code and know where shit comes from even when I'm reading it on GitHub.

vale otter
#

using only GL32C or GL46C would fall into the same issue IMO.
looking at the existing GlConst patch, the prepend of org.lwjgl.opengl.GL32 adds nothing to what it is that the GL_ or gl prefix (and if applicable ARB, EXT, or KHR suffix) doesn't.

#

also, small note, do make sure to use the C variant, GL32 is compat profile, GL32C is core profile

#

with GL calls specifically, doing ^ helps nothing and only makes reading it worse.
this is marginally helpful, but even still only marginally, because it actually tells you what version of GL its from

case DEPTH32 -> org.lwjgl.opengl.GL11C.GL_FLOAT;
case D32_SFLOAT_S8_UINT -> org.lwjgl.opengl.GL30C.GL_FLOAT_32_UNSIGNED_INT_24_8_REV;
late lotus
#

Another benefit of patching in the validation directly in the source code is that it can be done later in a non-breaking way. And potentially less invasive way if we only perform the few checks that actually matter in practice

vale otter
#

adding the validation is non-breaking regardless

#

its just an interface layer thats disabled in prod anyway

#

this also isn't about the validation, but rather NeoGlDevice's existence

fossil isle
# late lotus Another benefit of patching in the validation directly in the source code is tha...

I'm not talking about the validation, I'd actually be in favor of keeping the validation layer separate. What I'm talking about are the custom extensions and whether they should be patched directly into the vanilla GlDevice or kept in a custom class extending the vanilla GlDevice, the latter of which requires making the vanilla class abstract (with too little justification in my opinion)

vale otter
#

personally, id be more in favor of a NeoGpuDevice interface for any extension, and then NeoGlDevice implements that (and extends GlDevice) leaving the base b3d interfaces/objects almost or entirely untouched

#

which is taking what i did a step further, rather than moving more stuff to patches

fossil isle
vale otter
late lotus
#

What is a NeoGpuDevice and why is this not an injected interface?

vale otter
#

something that doesnt currently exist

#

but how i would personally handle where any added function to the interface gets added, instead of patching it into GpuDevice directly

late lotus
#

Why do things differently than our standard pattern?

#

I was very confused by this class, especially compared to NeoGlDevice

fossil isle
#

Putting the abstract extension methods into a GpuDeviceExtensions interface instead of patching them into GpuDevice is perfectly fine and in line with the way we do it everywhere else. My argument is literally just about their implementation in the backend-specific GlDevice vs. an extension thereof which requires making a vanilla class abstract

late lotus
#

Ah yeah I'd just patch them in GlDevice directly

vale otter
#

is the main problem making GlDevice abstract?

late lotus
#

Understanding the hierarchy of what you propose took me a while, it's much cleaner to patch directly the methods into vanilla

vale otter
#

the way i suggested would fix that, because GlDevice wouldn't implement the GpuDeviceExtensions/NeoGpuDevice interface

late lotus
#

As a side note using Neo(Forge) in the name of class should be avoided, it's non-descriptive and generally means that the design can be improved

#

(except for use cases like datagen of course)

fossil isle
vale otter
#

could patch RenderSystem#getDevice to return the extension interface instead

late lotus
vale otter
fossil isle
vale otter
#

the main thing i want to avoid is implementing everything in patches

late lotus
#

That is really not a problem though

fossil isle
#

I don't see a particularly good reason to avoid that. In the end it's a single patch hunk (or two if you also need to add fields), it's not like we'd be spraying lots of tiny patch hunks everywhere across the whole class

keen pine
#

Well one thing I see as majorly annoying here

#

Is that with it being a patch it requires him to use FQDNs everywhere

#

Which tanks readability of patches

#

Especially if this starts growing

#

For a method or two that is fine

vale otter
keen pine
#

For anything more then that, I would highly suggest moving the implementation out of the patch either into the default methods of the interface

vale otter
#

patches also have worse visibility into the code's history

keen pine
#

Or into a completely different implementation

vale otter
#

for anything that isn't trivial, good code history is a useful thing to have

keen pine
#

Also documentation in patches is awefull

fossil isle
keen pine
vale otter
#

note: i do use classnames for Java things, its with GL/VK where the spec required prefix is also there

#

IMO, GL46C.GL_ tells me nothing more than just GL_ does

#

the FQDN same thing, but longer

#

however, my own code has GpuTexture.USAGE_CUBEMAP_COMPATIBLE, because there is no other prefix on that

#

GL46C.gl* same idea vs just a gl* function call

fossil isle
vale otter
#

i mean, even i dont have it memorized, mostly, lots of googling and checking against spec

#

i also have less issue with GL46C vs the entire FQDN for it

vale otter
late lotus
#

yes

vale otter
#

oop, wrong link on that one fixed link

late lotus
#

final var is not a problem just ugly

vale otter
#

i mean, java doesn't have val

late lotus
#

just var is a lot nicer IMO

vale otter
#

i have a preference toward using final when possible, it can help for performance but usually for me its making it clear that the reference/value wont change

#

yes sure here it doesnt really matter, but i do this basically everywhere

#

i dont really care either way (C++'s const doesnt work the same way, so i often cant at work), just a preference (hence doing it in the first place)

late lotus
#

final is useless on function parameters and local variables; I'd even argue that it's harmful for readability of the code. I can't force you to change it so feel free to mark it as resolved

vale otter
#

its not useless though, it does still enforce that you cant re-assign it, and i have seen the JIT be able to pull of fairly large performance gains because of it

autumn zephyr
#

citation needed on that.

late lotus
#

not on local variables or parameters for sure, wrt. JIT

vale otter
#

granted, said perf gains were some hella math intensive code, but still

autumn zephyr
#

unless I am mistaken, final modifier on local variables doesn't even make it into the bytecode

vale otter
#

idk, i just remember seeing a perf gain from slapping final on everything i could

late lotus
#

well that is precisely what we are pointing out; that this does not seem true

late lotus
vale otter
#

50/50 that wasn't from the compiler but rather from me changing something that let the compiler make some new assumption it could, ill re-run and see on that, im curious on if this matters or not

vale otter
#

it is a lot of boilerplate, but i think keeping it as its own layer is the better idea

#

eventually, some more invasive validation may also be done, where keeping that in its own layer would be better anyway

late lotus
#

idk, this is a lot of code that does not need to be done in this PR

#

I have noticed that you tend to get carried away in your PRs it seems; this is not good for the poor reviewers like me who have to understand why the change is needed

vale otter
#

thats probably a habit i picked up from work, where its the exact opposite problem
single CRs are preferred, so its reviewed at once as a whole

#

ie: this is a related thing, instead of a second CR to bug everyone about, ill just shove it in this one

late lotus
#

don't you have the concept of reviews?

#

because big PRs are extremely annoying to review imo 😛

vale otter
#

CR: Code Review (or Change Request)

late lotus
#

ah right

vale otter
#

i can change it and/or break this PR up, but explaining my thought process

properties/features exists for enforcement, so the validation layer is there to use those (and enforce them)
features has logicop as the only one, and is given as an expected-usage example
properties has depthZeroToOne as a usage example of something where other parts of the game may need to react to a GpuDevice property
other properties are covering extending B3D (validation layer checking that), or existing B3D features that it doesn't check itself already, and serve as examples for how other properties may be used

late lotus
#

so, my reasoning is that we have a very clear scope for the PR, namely providing the ability to extend B3D in a non-breaking way in the future

#

adding additional properties such as depthZeroToOne should be doable later, and needs to be reviewed properly based on proposed use cases

#

I would expect validation exclusively for the new B3D features, at least in the first step; this is simply to ensure that we can extend B3D later in a sane way

vale otter
#

i can pull those things then

vale otter
#

also, this has at least 3 conflicting ways people would like to go, https://github.com/neoforged/NeoForge/pull/2399#discussion_r2190675964

it is mostly boilerplate in the current PR but, preferences are
xfact: definition in GpuDevice and implementaiton in GlDevice
tech: definition in new GpuDeviceExtension interface and implementation in GlDevice
me: definition in new GpuDeviceExtension interface and implementation in new NeoGlDevice class
and then what the PR currently has: definition in GpuDevice and implementation in NeoGlDevice

fossil isle
#

I never said the abstract methods should be in GpuDevice, I was specifically arguing about putting the implementations in GlDevice. I'm in the same boat as Tech that the abstract methods should be in an extension interface

vale otter
fossil isle
#

That was referring to the approach of having NeoGlDevice implement the extension interface instead of GpuDevice extending said interface which would have lead to the extension methods being "invisible" unless you already know to cast to either the extension interface or NeoGlDevice first

vale otter
#

i mean, if GlDevice implements the extension interface, largely the same problem

#

and also, the validation device's existence will prevent people from casting to a *GlDevice anyway, at least in its current form

fossil isle
#

Not nearly as bad, because auto-complete will suggest the extension methods on a GpuDevice variable/field/whatever

vale otter
#

oh, wait, you mean having GpuDevice extend the extension interface, rather than the other way aroudn?

fossil isle
#

Yes, as we usually do

vale otter
#

so, GpuDevice extends GpuDeviceExtensions that GlDevice then implements?

fossil isle
#

Yes

vale otter
#

ok, yea, that i can see

#

i was thinking the other way around, i see no reason to not do that actually

#

i think theres currently some functions in CommandEncoder and/or RenderPass, those should probably be pulled to a similar extension interface, but that can be done later

#

so, then its just where the implementation goes

fossil isle
vale otter
#

yea, this stuff probably is by itself too, but ive got a long enough list of things to add that it wont stay that way

#

which is where my argument for i dont want to implement this all in patches comes from, that list isn't all that short, and i know vaguely what implementing it will look like, and doing that in patches will suck

late lotus
#

As a middle group we can add the extension interface but do the actual implementation in a subclass of GlDevice

vale otter
#

thats basically what i was wanting to do

#

it being a super interface to GpuDevice changes very little to me

late lotus
#

It changes a lot for users

#

I think I'd still go for a big patch in GlDevice for now

fossil isle
late lotus
#

We can always move it out in another BC window

vale otter
#

definition in GpuDevice vs GpuDeviceExtensions (with GpuDevice extending GpuDeviceExtensions) makes basically no difference for the implementation

#

for the implementation, thats in GlDevice or a sub-class of GlDevice

late lotus
#

Users are the mods who want to call the new methods

vale otter
#

yea, and it makes little difference for them too, in either case if they have a GpuDevice, they can call it, i dont think we actually disagree on anything with the interfaces that mods will actually use

late lotus
#

Btw the convention is Extension without s 😅

vale otter
#

noted, will make sure to use GpuDeviceExtension when making the interface

#

i just dont want to put the impl in GlDevice itself via patches

late lotus
#

I think we can start with a patch and potentially move stuff later if it gets unwieldy

vale otter
#

IME, its not an if, but when

#

compute shaders in particular i foresee being unwieldy

late lotus
#

Well let's see. Yagni etc. It won't matter until we port again in which case we'll be able to move the methods

vale otter
#

i mean, ive implemented them before, and a whole "ComputePipeline" is going to be fun to try and impl in patches

yes i know the "ComputePipeline" object wont be in patches, but the GL impl part would be.

#

particularly because compute can be dispatched from inside or outside a renderpass (GL doesn't care, but the existing renderpass execution restrictions are pretty directly tied to what VK does)

hushed stump
#

Now, final on methods is super useful, the jit does use that as a hint for inlining

vale otter
#

@wraith osprey I sorta think that requesting extensions is out of scope for the event in Neo. The only case where you need to enable an extension specifically is with VK, and with that only if you are directly using VK as well. Given that the use case is asking Cinnabar to enable something, I think it would be better to have Cinnabar expose this instead.

wraith osprey
#

It's designed mainly for mods which need, e.g. compute shaders

#

It has been requested before from what I understand

#

(and also from what I remember, I don't remember where/when/how though)

vale otter
#

I plan to add compute shaders to B3D directly, so no need to ask for the GL extension

wraith osprey
#

ofc, but there are other extensions which would be nice to have too

vale otter
#

GL also doesn't require the extension to be enabled, so similarly no need to ask

wraith osprey
#

but they may not be present on e.g. macos

hushed stump
#

I think giving, encouraging, or generally making it viable to do direct GL/VK calls is just asking for trouble, mods should do everything through a b3d exposed api

vale otter
#

^

hushed stump
#

I dont want mods suddenly exploding when running on VK, or vice versa

#

or, oops sorry we only support dx12

wraith osprey
#

I would normally agree, but there's no way we can be fully prescient what people will need access to, so we should design a safe escape hatch which lets people break out if they absolutely need to

hushed stump
#

I think if people want to do that, they can do it unsafely with mixins

#

Its an incredibly hard line to draw in the sand, i'm sure some other maintainers share your viewpoint, usually i'd be on the other side here

vale otter
#

the existing interface says if you are on GL or not
For GL, there's no need to ask
For VK, Cinnabar can let you ask for extensions

wraith osprey
#

there is a need to ask on OpenGL because not all extensions are always present

#

like I said on macOS as an example

vale otter
#

You should be very aware of the backend being run on when doing something

#

Throw a mod loading exception yourself if the extension isn't present then

#

Same as you'd need to currently

hushed stump
#

i thought with mac you got ether 2.1 legacy profile, or 4.1 core

#

there was no mixed

vale otter
#

2.1 compat, 3.3 core, or 4.1 core

hushed stump
#

i didnt thnk there were extensons to enable

vale otter
#

Depending on hardware

hushed stump
#

mac gl is cursed af, your probably right there

vale otter
wraith osprey
#

it doesn't have stuff like mesh shaders though, plus the combined depth/stencil attachment is TECHNICALLY an extension which we check for

hushed stump
#

it has the concept of profiles, which can present different extensions

vale otter
#

VK has the idea of instance extensions, device extensions, and needing to explicitly enable them too

hushed stump
#

usually they are forward available, on older profiles

vale otter
#

Everything 3.3 and up are just making extensions part of the core spec (asterisk on DSA, there's one function thats weird)

hushed stump
#

I'm pretty sure mac gl was particularly cursed here, if you requested 3.2 you got ONLY 3.2 and nothing newer

#

unlike windows/linux

vale otter
#

No, that's Nvidia that does that

#

Mac has two request modes, 3.2 or 4.0, for core.
3.2 will give you 3.3, 4.0 gives you 4.1

#

I can ref the GLFW code that does the context creation

#

Or the CGL docs on it

hushed stump
#

I don't really want to argue it, whatever

vale otter
keen pine
#

Yeah sure

vale otter
#

oh, i meant specifically monica's ask

keen pine
#

There are for sure scenarios where you can use a fallback rendering route

#

When an extension is not available

hushed stump
#

I still dont think there should be any defined api for mods to directly use GL/VK, thats asking for trouble

keen pine
#

Yeah

vale otter
#

I'm mostly against it because it encourages reaching around, and that if thats being done it should be done with a more direct hand to the backend being reached around.

keen pine
#

Well I am on the fence

#

It is a reasonable ask

#

But if that circumvents the original concepts

#

Then it is a no

vale otter
#

Reasonable ask, but i think its better to put it as a ask the backend direrctly thing, rather than the generic API
the generic API should stay generic

keen pine
#

Okey

#

Valid point

#

Happy to be your sounding board here

#

I think it works either way

vale otter
#

Directly asking Cinnabar to enable some extensions is fair, as to do so you must know exactly what backend is being asked to do this

keen pine
vale otter
#

rather than any VK backend (though theres unlikely to be a second) responding to an extension request, when it may not behave the same and may break when reached around in that specific way

#

reaching around GL is also where state machine issues are liable to poke their ugly head up, and even i have been at fault for those.

#

making sure everything is always cleaned up properly is a PITA at best, and human error is human error.

autumn zephyr
#

you can always instanceof-check the device and do backend-specific stuff if you must, yeah

#

and btw, the extension strings are backend specific, correct?

keen pine
#

Yeah

autumn zephyr
#

Yeah then I'd not put that on the generic interface or event

keen pine
#

Okey

#

Agreed

#

Reasonable to me as well

wraith osprey
#

Fair enough

#

I felt the concept was general enough that it seemed reasonable to put it on the generic interface, and a backend can fairly trivially detect a "bad" extension.

autumn zephyr
#

yes but the consumer should pre-check if they're working with a backend they can reasonably expect to have that extension first
but if they do that, then they can just call a backend specific method, I think that will hopefull lead to more graceful degradation than outright crashes

wraith osprey
#

Seems good enough to me

fossil isle
#

One thing that came to my mind last night with respect to the debate about patch vs. subclass for implementing the extensions: since GlCommandEncoder and GlDevice are (mostly) stateless, the extension methods could also be implemented in an extension interface that's implemented on GlDevice:

interface GpuDevice extends GpuDeviceExtension { ... }
interface GlDeviceExtension extends GpuDeviceExtension { ... }
class GlDevice implements GlDeviceExtension { ... }

If an extension method does need to retain some custom state or touch any vanilla state, then getters and/or setters for those fields could be declared in the GlDeviceExtension interface and implemented in GlDevice via patches.

vale otter
#

what is the benefit to doing it this way as opposed to making GlDevice abstract and extending it?

fossil isle
#

Not having to make GlDevice abstract

vale otter
#

what is the drawback of doing that?

hushed stump
#

I think we generally try and keep bincompat with vanilla, but that line is fuzzy these days i think

vale otter
#

i guess its mostly that im considering GlDevice an internal API

hushed stump
#

Is the class public?

vale otter
#

it has to be because RenderSystem makes an instance of it

autumn zephyr
#

there are no "internal" APIs in Minecraft

hushed stump
#

Then its public mc api surface

vale otter
#

but thats the only place anything in that package is referenced by Mojang iirc

fossil isle
hushed stump
autumn zephyr
#

I honestly got a bit lost in the scope of all the PRs being opened.... Can you give a summary of the goals we're trying to reach?

#

The initial one on my radar was adding missing GL state to Blaze3d

vale otter
#

2399 is get the boiler plate in place so that adding stuff to B3D isn't a breaking change
this comes along with the validation layer to check usage, and mechanisms for reporting and enabling things that may or may not be supported by the backend being used

autumn zephyr
#

when you say "adding stuff to B3D", what kind of additions are we talking about, and who'd be doing the adding?

vale otter
#

so, what this allows is for something like indirect drawing, and any required usage bits, enums, functions, etc, to be added while Neo is stable, and if the backend doesnt support it, it reports it doesnt support it, and calling any of those functions or using those bits/enums is considered invalid (and the validation layer in dev will catch you doing this)

#

note that "reports it doesnt support it" is more "doesnt report it does support it" because doesnt support is the default state

#

this is a very similar style to how VK handles extensions, with GL being vaguely similar too (just no need for explicit enabling)

#

the other benefit of the validation layer is that in the future profiles for it that match different known hardware could be made
that would allow for testing in an emulated environment more restrictive than the hardware used for dev, without actually owning that hardware

autumn zephyr
#

Ok, so the goal is to establish an API contract now for backends (which is still breaking Vanilla compat) that establishes they need to handle extensions of certain enums gracefully

#

Not so sure about the validation layer and how that plays into that

vale otter
#

both the backends need to, and mods need to

#

old cinnabar doesn't know about USAGE_INDIRECT
new mod does, and uses it
the validation will check that you enabled the indirect feature when using the USAGE_INDIECT bit, in dev
at runtime, if you try and enable that feature, and Cinnabar doesn't know about it, there will be an exception thrown (which the validation layer enforced that you asked for)

#

the validation is entirely an enforcement mechanism to make sure mods (and backends) aren't breaking the contract

autumn zephyr
#

I don't know if the validation layer isn't complete overkill, tbh.

vale otter
#

if theres no enforcement, then mods are liable to crash later, rather than at startup

#

something like Quartz could never make an indirect draw call and everything is fine, but the moment it does everything explodes because Cinnabar doesn't support that, and there was no check nor enforcement of that check

#

VK in particular also has this fun thing where unless a feature is enabled ahead of time, its not available. So this means that even if the version of Cinnabar does support it, if the featue wasn't enabled it may not be available, and now you get a native level crash instead

#

GL doesnt suffer from that one, if GL advertises it, its enabled and available

vale otter
#

@fossil isle 2399 poke

vale otter
#

Could a check for using the right license header be added to the immaculate checks?
I know there already is a license check, but i didn't even notice that there were two different headers

#

at some point i copied one, obviously got the wrong one, and then just copy-pastaed it again and still had the wrong one

fossil siren
#

no, it's intentional so we keep the forge ones where they already exist

vale otter
#

only for new files*

fossil siren
#

immaculate doesn't know what is a "new" file is the problem

vale otter
#

i was going to suggest however tech did it for interfaces, but just listing all the files that are allowed to have the old one probably isn't reasonable to do...

vale otter
#

@fossil isle i think i got them all

fossil isle
vale otter
#

thats exists? cool, ill use that next time

#

i have a feeling ill be adding more files at some point

fossil isle
#

2399 looks fine to me now except for one point: we should provide a way to disable the validation layer in dev (could be done with a system property), for example to remove the overhead when doing performance testing.

vale otter
#

ie: if System.getProperty("neoforge.disableB3DValidation") turn it off?

fossil isle
#

Yes

vale otter
#

i thought about this when adding the prod check, and then promptly forgot to actually add some mechanism to force it off in dev

#

so, off if (FMLLoader.isProduction() || System.getProperty("neoforge.disableB3DValidation") != null) {

fossil isle
#

I would do if (FMLLoader.isProduction() || Boolean.getBoolean("neoforge.disableB3DValidation")) {

vale otter
#

does that work where just -Dneoforge.disableB3DValidation turns it off?

#

the != null does, because that returns an empty string

fossil isle
vale otter
#

i mildly prefer being able to just define it, but only mildly

#

so, Boolean.getBoolean it is.

#

pushed up

#

also, do you have any particular thoughts on 2445?

fossil isle
#

Not really, no, in part because I lack the knowledge to understand what that bit would actually do under the hood

vale otter
#

ahh, it tells me that i can use a temporary buffer to back it, and dont need to track that frame to frame

#

for entity rendering, that means i can use a bunch of them, upload all of them from the CPU at the start of the frame, and then use a single continuing renderpass for all entities later in the frame

#

rather than stall, upload, stall, draw, stall, upload, stall, repeat

#

without that bit, sync requirements make it a lot harder to try and do things like that

#

GL can also utilize it by having a much larger persistently mapped buffer(s) backing it, and using a new section of it on each upload, to similar sync benefit

#

its a very neat trick thats helpful for how MC does entity rendering in particular, or any staging buffer

vale otter
fossil isle
#

Hmm, interesting

vale otter
#

@late lotus @keen pine poke for 2399 because requested changes
cc: @autumn zephyr too if you want to actually look it over

#

in general, i dont expect mods to use it, but they also probably don't need to

#

Nvidia and probably AMD will allocate multiple buffers behind the scenes, and do magic with them to avoid most of the cost. I tried something similar, it didn't work out well. Could probably try and make it better, but id rather not try and replicate the Nvidia driver.

fossil isle
#

This does make me wonder again what the Qualcomm windows driver is fucking up to require that staging buffer workaround

vale otter
#

likely a sync thing

#

using a single buffer for upload is likely making it stall a lot, which solves the sync issue

fossil isle
#

🤔

vale otter
#

i say that because Cinnabar had neither issue on Qualcomm hardware

#

but Cinnabar has to do its own syncing

#

logicop also works fine, because reasons

#

also, isn't it 3am for you?

fossil isle
#

Yes 😅

vale otter
#

do you just, not sleep?

fossil isle
#

My sleep schedule is completely fucked

vale otter
#

understandable, have a nice insomnia

woeful oyster
#

@vale otter ok, here is the code I was referencing

uses some util functions from this file as well

and it uses this shader with this include

the TLDR is that there is an in-game editor for the dragon players in the mod, and the end user can select between hundreds of different parts, each with their own hue/saturation/brightness; what this does is take all of the user's skin data, and then dynamically render the individual textures into a "master" texture on the GPU for each player, which is then cached

just wanted to know if there were any major red flags for the new B3D API

vale otter
#

embeds

sinful lion
#

embeds yeeted

woeful oyster
#

let me also pull up a quick screenshot of what this editor even looks like for some extra context

vale otter
#

as it is right now, it looks like you are going through the old RenderSystem GL manager and old B3D, you are likely already covered by existing B3D

#

some things will need to be updated, but feature wise you should be covered

woeful oyster
#

oh ok great

vale otter
#

it should be a relatively straightforward port, its mostly just defining stuff ahead of time in a RenderPipeline instead of directly setting stuff in RenderSystem

woeful oyster
#

so the new stuff is stateless (like vulkan)?

#

since I had issues with this setup in the past where I completely messed up the state of stuff (regardless of my attempts to try to restore the GL state) so I just forced this to all happen at the start of the frame rather than on-demand (which was how it originally was setup)

vale otter
#

pretty much, yes

woeful oyster
#

awesome, my life is becoming a lot simpler then cryingintheclub

vale otter
#

its notable that a VK impl was alluded to, that already exists (and I'm the one that made it), which ofc is completely stateless

#

it is a very vulkan-like API, not perfectly, but enough to make that ^ possible

#

one thing i will say, dont yeet shit every frame

# 
        glowTarget.destroyBuffers();
        normalTarget.destroyBuffers();
#

dont do that

#

you should re-use stuff like that for many frames

woeful oyster
#

well in this case the size of that target might change

#

depending on the textureSize of the DragonBody

#

so i wasn't really sure what to do

vale otter
#

make a bigger than necessary one, and set the viewport/scissor

#

then blit the subset you used

woeful oyster
#

the problem is that when it is time to render the dragon it expects the texture to be that size

#

as in, not have some of the data be a subset of a texture

vale otter
#

if (size < needed size) { recreateTexture() }

#

you could also cache multiple sizes

woeful oyster
#

i guess i could do it at the time of the copy

#

anyways i'm aware there are ways to fix this, i'll figure it out if i have the will and time at some point in the future

#

the thing is, we're not really yeeting this every frame in reality

#

since this generation code is only called when a new player joins with skin data, or an existing player changes their skin data

#

which happens very infrequently, other than on the editor screen

vale otter
#

oh, ok, thats probably fine then

#

i assumed this happened every frame

woeful oyster
#

it happens every frame in 1 case, if the user is sliding the color bar in the editor

#

so it smoothly updates the texture as the user is changing hue/saturation/brightness

vale otter
#

maybe cache whatever the last used texture was

#

in that case, it should always be the same size, so the same one can be re-used

woeful oyster
#

i suppose

vale otter
#

creating/deleting textures is expensive to do

#

buffers too, but i dont see you making buffers, just textures/framebuffers

woeful oyster
#

in any case, this is miles better than the previous implementation that was done, where it was done as a blocking CPU-side copy, the whole game just stalled to 10 FPS when you moved the color bar before cryingintheclub

(i rewrote a lot of the old code for this mod to get rid of crazy stuff like this)

woeful oyster
vale otter
#

your refactoring should be pretty simple luckily

#

mostly just making a render pipeline instead of a shader instance, and then shoving all your uniforms into a UBO

vale otter
#

@fossil isle i forgot about doing this earlier, do you want me to open a PR to move the stencil state to the RenderPipeline, or do you want to do that, or someone else?

if its not moved id need to check hardware compat for VK_EXT_extended_dynamic_state because VK_DYNAMIC_STATE_STENCIL_TEST_ENABLE and VK_DYNAMIC_STATE_STENCIL_OP aren't part of VK 1.2, which might be a problem

#

so far, ive just had the functions throw, but ofc if its not being used thats never going to be hit so i forgot about it

vale otter
#

got a prototype of moving the loading overlay to using a B3D backed APIRenderer working. was pretty easy to do, but also uncoverred a bug in B3D, go figure

vale otter
#

ill get PRs for that bug and for moving the stencil state open tomorrow
also need to add viewport to the renderpass, didn't realize that wasn't supported, but thats easy enough to do

#

^ the loading overlay uses viewport, hence needing that

fossil isle
fossil isle
#

@vale otter one more thing regarding 2399: please adjust the PR description to match the adjusted scope of the PR, primarily with respect to the removal of the depthZeroToOne property

vale otter
#

what is the stencil test supposted to look like when its working correctly?

#

this doesnt seem correct based on the description

image.png
#

ok, thats the exact same as what happens w/o my changes, @fossil isle, is the test broken or is that expected?

autumn zephyr
#

It's quite possibly broken in 1.21.7

#

I don't remember what we did to try and fix it

vale otter
#

yea, i checked in renderdoc, the grass doesnt even get rendered

autumn zephyr
#

How did we even try to fix it? PIP?

#

it kinda has to be a PIP

#

since that's the only way to interleave raw render-calls, but funnily enough that also means only the off-screen render target will have stencil use

vale otter
#

i mean, w/ pipeline modifiers, if it can hold the modified pipeline for the render later, that would work

autumn zephyr
#

Hmmmm, I vaguely recall talking about having to apply pipeline modifiers as render-state is being recorded for the GUI but I don't remember if we followed up

#

(which requires GuiGraphics to have its own modifier stack IIRC)

vale otter
#

yea, probably would need to mirror the scissorStack

autumn zephyr
#

yeah

#

Although we wouldn't need to capture it in the actual state

#

we could simply apply the transform as the pipeline is stored in the state

fossil isle
autumn zephyr
#

Yes, it's more likely to be used in-world

#

I really meant more in this particular test case

#

IIRC the only user of stencil was Immersive Engineering wire-rendering?

#

Or am I imagining things

fossil isle
#

As for stenciling in UIs: I'm not sure it's worth supporting them on elements directly since you'd need to render two elements, one that prepares the stencil and one that uses it for masking, and relying on these elements staying in order (especially when there's more than one use of stenciling on screen at once) is a fools errand in my opinion

autumn zephyr
#

Probably, yeah, the pipeline modifier is just a more general thing

#

Especially for item rendering in the UI

#

but I'd also expect stuff like this to be PIP

fossil isle
#

The pipeline modifier system would still be relevant for this within a PiP. Only downside is that you need to flush the buffer yourself within renderToTexture() instead of leaving that to vanilla

vale otter
#

should fixing it be included as part of PR to move it to the pipeline, or because the test is currently broken its fine to leave it updated to use the modifier system but still broken?

fossil isle
#

Would be nice to fix it since you need to modify it anyway

late lotus
#

Which PR? I hope not 2399

vale otter
#

no, not 2399

#

new one, doesnt exist yet

#

moving stencil to the render pipeline, rather than dynamic state on the renderpass

vale otter
late lotus
#

Not gonna comment on final var but please don't use it in future PRs 😫

autumn zephyr
#

i mean we don't have a style rule on it but i'd also prefer not to use it

vale otter
#

a rule on it should probably be added then

#

or, i guess more generally than final var is that its final on a local, because final GlDevice is the same thing there

late lotus
#

Why are Properties and Features still different concepts?

#

There's really no difference on the user side IMO

autumn zephyr
#

Without looking at the code: property == value range, feature = on/off?

#

such as: how many texture units does the GPU have, blah blah

vale otter
#

^ that is more or less correct

#

not that it needs to be done this way

autumn zephyr
#

properties are read-only I'd assume since they're just information about the device the user has?

#

where the spec leaves wiggle-room such as "minimum of 4, up to X" of some thing

#

At least that's how it was for GL

vale otter
#

yup, though there can be two different sets of properties, theoretically available (during device config), and then actually available (varies by feature enablement)

autumn zephyr
#

you mean additional properties or the value of properties can change?

#

I'd find that surprising

vale otter
#

ie: if extendedDepthTestFunctions is disabled, the extra depth test enums aren't reported (note: doesnt exist currently) after configuration

#

so properties after config tells you what you can actually use, where during config is what you could use

autumn zephyr
#

now that's confusing

late lotus
#

Some properties need enabling

#

So there's no real difference for users IMO

autumn zephyr
#

Are you sure? It sounds more like you need to enable a feature to potentially get more values in a property

#

but as far as I understand it, some features will add a minimum set of new values, but depending on device support may add more, hence the potential need to inspect the properties for support (you should confirm that, RogueLogix)

#

and maybe we should make that a bit more concrete

#

But I find the use of properties before feature requests a bit odd

late lotus
#

We discussed that anything beyond standard OpenGL core 3.2 needs explicit opt in

vale otter
#

for reference of why its separate in the first place, and where the ideas come from
properties is following the idea of VkPhysicalDeviceProperties which you fetch from the physical device before logical device creation
features is following the idea of VkPhysicalDeviceFeatures which you fetch from the physical device before creation and then pass a subset of to logical device creation to actually enable things

late lotus
#

I think the separation is quite artificial

#

And I don't think that doing things because that's how it is in vulkan, makes sense for what we're doing here

autumn zephyr
#

So from a modder perspective

#

Okay no, before I continue, I'd want to see what are examples for properties. I'll have to look at the PR I guess

#

Which one is this in?

#

2399?

vale otter
#

2399,

late lotus
#

Examples might be LogicOp or some GL 3.3 dest factor

autumn zephyr
#

Ok

vale otter
#

or the number of texture units available, that would be a property

#

without a corresponding feature

late lotus
#

Yeah but that's not really in scope of what we're doing here IMO

autumn zephyr
#

I'd expect logic op to be present only if there's a HW support difference

#

If the only difference of supported logic ops is whether feature X is requested or not, but the supported ops are either fully supported as requested or not

#

then that doesnt need to have a property

vale otter
fossil isle
vale otter
#

vice versa is also going to be common, and also many properties for one feature

autumn zephyr
#

well there can be overlap between feature and property

#

just as a thought experiment

late lotus
autumn zephyr
#

the GL 3.2 spec says: LogicOps A, B and C are required by 3.2, but devices may support D, E, F

#

and then there might be an extension that adds G, H, and I

#

that'd be a case where you have a property + a feature

autumn zephyr
#

you don't get to ask for more

late lotus
#

That sucks from a modding pov though

autumn zephyr
#

Which one?

late lotus
#

The point is that the modder acknowledges that the feature is only available on some hardware and will have a workaround in place if it's not

vale otter
autumn zephyr
#

Yes, which is why I think properties you don't need to know before config (?)

#

Is there a use case for knowing the properties before config, i.e. to ask for a potential extension if the device doesn't support another optional feature?

#

I can see such cases

vale otter
autumn zephyr
#

i.e. to implement alternate render paths if the device doesn't have a native feature

fossil isle
autumn zephyr
#

12 is good enough for anyone!

vale otter
#

ie: i need compute and 24 textures, else i care about neither

vale otter
late lotus
#

My point is we want modders to opt into non standard "things"

autumn zephyr
#

yeah yeah ok ok, but we should limit properties to the ones that actually change with HW suport

#

the rest is superflous IMHO

late lotus
#

Splitting these things across two interfaces is not good

autumn zephyr
late lotus
#

There's no difference from my pov

autumn zephyr
#

there is

#

you can't opt-in to something that doesn't exist

late lotus
#

Okay so?

autumn zephyr
#

you only use these properties as a decision point to use an alternate rendering implementation

#

if the device simply doesn't have what you need

late lotus
#

Well same as extensions

autumn zephyr
#

as I said, read-only vs. non-read-only

#

no, since you can request an extension (write access essentially)

late lotus
#

If the device doesn't have the extension you have the exact same problem

autumn zephyr
#

while you cannot request more properties

#

bad wording

#

you cannot request more support from a property

late lotus
#

I am operating under the assumption that we don't want to add extensions to B3D that might not work for some hardware unless the modder opts in

autumn zephyr
#

wrong assumption then

#

GL 3.2 already has such variance

late lotus
#

?? B3D does not though?

vale otter
#

b3d takes in GLSL, it does

late lotus
#

So you might also be making the wrong assumption 😛

autumn zephyr
#

you're only thinking in on/off features

#

I am thinking about integer ranges 😄

late lotus
#

No, I am thinking a lot more generally

vale otter
#

features are all booleans

#

on or off, if theres mutitple levels, thats separate features

late lotus
#

I don't care if it's a bool, an int range or a set of flags, it's the same concept that the hardware might or might not support it

autumn zephyr
#

Now you're forcing me to look it up 😄

#

God this spec is seriously from 2009

fossil isle
# autumn zephyr yeah yeah ok ok, but we should limit properties to the ones that actually change...

Keep in mind that there are two layers of support involved here, one being the actual hardware support and the other being what the backend implementation supports. The point of this abstraction is not just to indicate what the hardware can support, it's also to make the addition of new B3D features (i.e. new enum values, new usage bits, new draw modes, entirely new systems like compute shaders, etc.) non-breaking for custom backends

late lotus
#

What's there to look up seriously

#

The idea is simple enough

autumn zephyr
#

Okay let's take maximum texture size

late lotus
#

If we add a GL3.3 feature to B3D do we have any guardrail to make sure modders don't accidentally lock out X% of the players from using their mod?

autumn zephyr
#

I don't see at a glance that B3D validates this for created GpuTextures

vale otter
#

0% in that case but idea applies

autumn zephyr
#

GL 3.2 guarantees 1024x1024 as the supported minimum size, but devices may support more

#

And since B3D doesn't seem to clamp it

#

You already have the situation that mods can use the HW capability

#

And max texture size is precisely something I'd expect to have a property for

late lotus
#

Well yeah this would be impractical to enforce. But it's also a different topic, and arguably out of scope for this one PR

autumn zephyr
#

Is it?

#

You need the property in a device-independent manner

#

erm

#

backend-independent, not device

late lotus
#

Yes, the PR is already large enough. I don't want more stuff in it

autumn zephyr
#

So that is true

late lotus
#

We'll do it in a second PR if we need to

fossil isle
autumn zephyr
fossil isle
#

Same thing for uniform offset alignment for example

vale otter
autumn zephyr
#

Yes

#

It's a vanilla method

vale otter
#

amazing

late lotus
#

So they do already have getters 🤔

#

Just after the device is created

autumn zephyr
#

okay I'd go with Tech here

#

on the following basis:

#

we can easily add properties later

#

Those should be purely informational

vale otter
#

Properties should be known during config IMO, specifically to inform decisions on what features to enable

#

And/or fail mod loading immediately during device config, rather than at some later point

late lotus
#

Yeah but mojang didn't make it so, so changing that is another discussion

#

Right now the goal of the PR is to allow opting in to B3D extensions, without adding extensions yet, and doing so in a way that future extensions can be added without being a BC for B3D implementations

fossil isle
late lotus
#

Yeah of course

#

Let's see in the future if more are needed