#Blaze3d Extensions (1.21.6+)

1 messages · Page 2 of 1

late lotus
#

The DestFactor example is a lot more relevant to this PR

#

Should non-standard dest factors require any opt-in from the modder, is the question?

#

@autumn zephyr

vale otter
#

then the class needs to stay

#

adding it later is a breaking change

late lotus
#

No, we're not gonna keep an empty class

vale otter
#

well it cant be added later, so it has to

late lotus
#

Adding properties is gonna be a BC anyway unless we default to the min spec opengl supports

#

Which would be a reasonable approach ig

autumn zephyr
late lotus
#

Non-Open GL 3.2 core

autumn zephyr
#

added by extensions?

#

Wasn't the entire point of this PR to require opt-in for that?

vale otter
late lotus
vale otter
autumn zephyr
late lotus
#

Don't confuse our extension of B3D (i.e. just adding stuff to it) and the OpenGL concept of extensions

autumn zephyr
#

Well true

vale otter
#

though there is overlap in that a b3d extension is likely to map to a GL/VK extension

autumn zephyr
#

Since the new blend factor will likely be implemented in a backend-specific way

#

you'd need to request use of the factor

#

not a feature

vale otter
#

feature: extendedBlendFactors
properties: whatever the factors are get added to the allowed list

fossil isle
late lotus
#

Ok yes that is fine. But we can add the properties interface in a non-breaking way with suitable defaults (e.g. new event constructor etc)

autumn zephyr
#

Eh I think I'll leave this up to you guys to iron out

#

Already too many cooks in the kitchen on this one

#

(And I'd first to have to actually read up on prior discussions and the PR while not having the time to do so)

fossil isle
late lotus
#

Oh the enums should be features not properties IMO?

fossil isle
#

I would agree with Rogue that features are purely flags while properties are the actual underlying value ranges. The former is modifiable (within a certain window), the latter is read-only

vale otter
#

if nothing else, while we dont need to follow what VK does, id at least prefer to be consistent with it for my own sanity

#

ie: not go directly against it and shove something thats a VK property in our features interfaces, or vice versa

late lotus
#

The extra SourceFactors require opt-in

#

So that should be an enabled feature, not a property?

vale otter
#

both

#

enabling them is a feature

#

what source factors you can use is a property

fossil isle
#

I think shader types would be a more relevant example. Compute shaders and task/mesh shaders would be two separate features but the overall supported ShaderTypes would be a property

late lotus
#

Surely you need to check if the feature is available?

#

And at that point what's the difference with checking if the property is available?

vale otter
#

properties aren't enableable

#

so, config will need to care about features (and properties) still, even if you could infer the feature from the property values later (or vice versa)

#

besides features/properties that dont have one of the other, or otherwise dont share a nice 1:1 relationship

late lotus
#

Ok to make progress let's see what enabling compute shaders would look like. What would the pseudocode look like?

vale otter
#

during device config, probably just enabling the feature, which would also need to implicitly enable other things like SSBOs

if (event.computeShaders()) { 
    event.enableComputeShaders();
}
#

though, may want to add this to that, because xfb can maybe be used in some circumstances to fake it, assuming xfb support is added to B3D

else if (event.transformFeedback()) {
    event.enabledTransformFeedback();
} else {
    // maybe throw?
}
#

and the backend supports it, Cinnabar likely wont even if Neo does

#

actually, compute probably wouldnt implicit enable anything, because SSBO and imageloadstore are both valid options, and ILS is broken on some GPUs and should probably be reported as not-available on them (terascale 2 on linux)

late lotus
vale otter
#

need, no

late lotus
#

Hence there wouldn't be a need for having the knownShaderTypes getter as it's redundant with the corresponding feature

vale otter
#

but you may care about the GL_MAX_COMPUTE_WORK_GROUP_COUNT properties or GL_MAX_COMPUTE_SHARED_MEMORY_SIZE property for if you are going to use compute at all

#

also, GL_MAX_COMPUTE_WORK_GROUP_SIZE

late lotus
#

Sure yes. Those I would put in the properties, but not whether compute shaders are available

vale otter
late lotus
#

Mostly or exclusively?

vale otter
#

a mod can query them and thats valid to do, but the intention is for the validation

late lotus
#

The validation layer can do a bit of extra work to check for the corresponding feature instead

#

Same for a mod

vale otter
#

the known bits is also useful for if a information usage bit (FRAME_TRANSIENT) is added, you can just mask your usage with the known bits

late lotus
#

Idk, seems a bit like trying to construct a use case to me

vale otter
#

to an extent, but i actually thought about doing that for 2445

late lotus
#

You'd only be able to do that for "hint"-type bits, not for anything changing behavior

#

So overall I think that we can just remove many of the current properties and instead check for the actual enum values in the validation

#

We can keep backendName and apiName basically

vale otter
#

for the usage bits specifically, id need a way to inform the validation layer of backend specific ones

#

i haven't thought about how on that at all, just stating the problem

fossil isle
vale otter
#

ie: VK_BUFFER_USAGE_SAMPLER_DESCRIPTOR_BUFFER_BIT_EXT is very Cinnabar/VK specific, and id shove in the top 8 bits (see 2359 explicitly reserving those bits for doign stuff like this)

vale otter
late lotus
#

E.g. if (shaderType == ShaderType.COMPUTE && !enabledFeatures.computeShaders()) // not good

vale otter
#

the enums are easy enough to do that with, but how should the usage bits get passed up?

fossil isle
vale otter
#

or should Cinnabar be expected to extend the validation device's calls that take a usage bit, check the cinnabar specific ones, and then neo will only check the lower bits?

late lotus
#

if ((bits & SOME_BIT) > 0 && !features.someBit()) // not good

vale otter
#

for neo added features, yes

vale otter
late lotus
#

We can always construct the bimask in the validation layer if we really need to make this faster

late lotus
vale otter
#

its always disabled in prod

#

though actually, force enablement in prod should probably be a thing

late lotus
late lotus
vale otter
#

and then should Neo mask off those bits in its own checks? because Neo will need to catch someone using bits that it itself doesn't know about yet.

vale otter
late lotus
#

I mean a literal toml config option

vale otter
#

so, still need a way to tell the default layer hey, this bit is ok, i know what it is and its ok if its set

vale otter
#

also, if this change is made, should properties be made immutable from before device configuration? if the validation layer is going to make assumptions, id prefer doing this.

ie: even if gpu atomics and/or compute shaders are disabled, it will still report that you can have 8 of them in your compute shader if thats what the underlying hardware allows

late lotus
#

I don't know if we should check the reserved bits. It would have to be specific to the GL backend by definition

vale otter
#

not checking those if the backend doesnt seems like a bad idea to me

#

maybe could just pass a boolean or something to the constructor to tell it to mask those off in its checks

late lotus
vale otter
late lotus
#

Yes I think that would make a lot of sense

vale otter
#

i think ^ is a good enough solution, keeps the validation of those on by default, but lets the backend say i got this easily enough

late lotus
#

Where is that boolean sourced from?

vale otter
#

passed to ValidationGpuDevice constructor

#

so, in my case it would be a super(cinnabarDevice, true) call, where in ClientHooks it would be new ValidationGpuDevice(glDevice, false)

late lotus
#

Ah sure

#

Good enough for now IMO

vale otter
#

is it fine if i leave some boilerplate classes in place because they will get utilized later? Specifically ValidationGpuTexture, i dont expect to need ValidationGpuBuffer for any reason hence not adding that.

#

also, <br> is used quite a lot in Neo already, i realize your comments starts with "nit" as is, but is there an actual style preference one way or another?

#

in some comments, i use both <p> and <br> for a larger and smaller line break

vale otter
#

@late lotus updates to validation layer and properties pushed up
docs and client hooks untouched

late lotus
#

LGTM, just two minor requests left

#

Explicit line breaks are not great IMO. Better use <p> if you want a line break, or other tags (e.g. <ul> for lists)

#

Markdown comments are coming soon enough anyway

vale otter
#

ill update the list at least, thats what i actually wanted anyway

late lotus
#

Ah shit I did change the actual config key

#

Might as well replace B3D by Blaze3D then when you get around to fixing it

#

Oops

vale otter
#

easy fix, i didn't notice that though

#

fixed and pushed

#

i did check it in game to make sure it grabbed it correctly, it did

late lotus
#

Ok approved. Will you go and fix the javadoc to use a proper html list?

vale otter
#

already did

late lotus
#

Oh right, nice

#

Just need XFact to approve again then we're good!

vale otter
#

@fossil isle

#

now that 2399 seems to be sorted, poke about 2445 and 2448, if you got time
also, other PRs ill be opening later today, but they are relatively simple

  • fixing a bug in the writeToTexture that takes an IntBuffer, it does the range check wrong (non-breaking aside from contract change)
  • setViewport/resetViewport to RenderPass (ELS overlay needs it, and mods are probably going to want it too)
    and then later can also update the loading overlay/FML, but thats a later thing because its completely non-breaking
#

TLDR of 2445, allows me to not constantly stall the GPU during entity rendering. Technically helpful for GL backends too because GL drivers are liable to do something similar to what Cinnabar does.

late lotus
#

I don't have more time today

#

You can merge 2399 however

vale otter
#

kk

austere wren
#

can we enforce the condition of USAGE_PERF_FRAME_TRANSIENT in software?

#

"may" crash sounds like a great way for mods to work fine on GL and then explode on VK

vale otter
#

Yes

vale otter
#

in reality, i would catch this and throw myself, but the validation layer could catch it too, if it knows when the end of a frame is

#

now then, with that merged, time for me to go to the other shit i need to do today

#

Which, actually, an explicit "end of frame" is quite useful to Cinnabar too, so, to enable the validation of that, probably is in scope to do that in 2445

vale otter
vale otter
#

draft #2470 opened for this, still needs the test fixed

vale otter
#

^ branch for that is on the main repo, so you can commit to it easily

fossil isle
#

I'll take a look tomorrow or so

late lotus
#

What's the status of earlydisplay on vk?

vale otter
#

i have a prototype of both the FML and Neo side changes, need #2471 for the Neo side
should probably wait until after the ELS error display gets into FML, then redo the prototypes more properly

vale otter
#

@fossil isle got an opinion on what do do in 2471 (https://github.com/neoforged/NeoForge/pull/2471) with resetViewport. Mojang doesn't pass the color or depth texture through to the object, so it has no fucking idea what the render extent is without asking GL or passing it through.

passing it through the constructor breaks bincompat
a secondary constructor can't really be used because that'd just leave the values uninitialized
setting it outside the constructor also runs into ^
asking GL is slow (probably)

the only reasonable option to me seems to be to just not have it and force the user to know the render extent and reset viewport accordingly if they need to
forcing a setViewport(0, 0, framebuffeWidth, framebufferHeight), which is all resetViewport really is anyway

fossil isle
#

Under which circumstances would you need to reset the viewport?

vale otter
#

not just reset, but reset without knowing render extent

#

i cant think of any where both of those are the case

#

particularly because if you set the viewport, you likely set it relative to the render extent

#

the state is also only held for the renderpass, so starting a new one will reset the viewport to the full render extent, no need to explicitly reset it there

fossil isle
#

Yeah, that's why I'm wondering whether there's even any need for it

vale otter
#

probably not, i was mostly just matching scissor, which does have a generic "disable" call

#

notable that cinnabar implements disableScissor as an enableScissor(0, 0, width, height) call, so this is a very similar situation

#

ima go ahead and remove it then, to keep bincompat with the renderpass constructor

fossil isle
#

👌

vale otter
#

tiny change now, +14-2

vale otter
#

w/ 25w37a bumping the GL version to 3.3, that adds API level support (w/o extensions) for

  • explicit attrib location in shaders (vtx in, fra out)
  • vertex attrib divisor (instanced attribs)
  • sampler objects
  • shader bit encoding
  • texture swizzle
#

two of those are in-shader, though explicit attrib can make the CPU code a bit nicer too (no names of attribs)

grim latch
#

ValidationGpuDevice subclass breaks things. Can we change it from inheritance to composition?
Even in NeoForge you can see lots of TODO 1.21.8: Can't require a validated wrapper since we initially forgot and that'd make it a breaking change

vale otter
#

what specifically is it breaking for you?

#

one of the main points is that it prevents you from reaching around it
you should not be relying on the default OpenGL backend being the backend in use.

grim latch
#

When we do cross-API interop, we must know the backend information, for example, for OpenGL backend we need to know texture id, for Vulkan backend we need to know VkImage, VkImageLayout, etc. Now it can be a subclass of ValidationXX, which is neither OpenGL nor Vulkan.

And yes, we're not relying on OpenGL. These interop codes handle every backend case instead of relying on OpenGL, e.g., CinnabarVK.

Not only that, we may also need to import external textures/buffers and wrap them into Blaze3D’s GpuXX classes. The validation wrapper classes break everything, they lack extensibility.

vale otter
#

if you are handling each backend's integration directly, then as was suggested for Sodium doing similar things, the fields exist

#

they are not considered supported/stable API surface, but realistically are unlikely to change.
the validation layer and it being in the way is to make it clear that you are doing something dangerous, that may break.

fierce ember
#

I don’t believe the command encoder has an accessible function for accessing the original still