Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support graphics cards without ARB_half_float_vertex #1179

Closed
wants to merge 6 commits into from

Conversation

illwieckz
Copy link
Member

@illwieckz illwieckz commented Jun 4, 2024

This is to address those two issues:

  1. Some graphics devices don't supports ARB_half_float_vertex like the Intel GMA Gen3 architecture with the Mesa driver.
  2. Some graphics devices don't supports ARB_half_float_vertex but drivers may provide an unplayable slow emulation, like the ATi r300 architecture with the Mesa driver.

Those changes purposes to provide an alternative code path when ARB_half_float_vertex is not available or when the r_arb_half_float_vertex cvar is disabled on purpose to avoid a slow emulation provided by the driver.

It is the next step over preparatory work that was already merged:

This branch doesn't break compatibility with the game but I implemented it over the illwieckz/image-fistcreen/sync branch anyway:

I did it that way because this other branch also helps running the game on this kind of hardware (to not upload textures too big for such hardware) and we better test this hardware over for-0.55.0/sync because of all compatibility-breaking changes purposed for performance that were merged there. The illwieckz/image-fistcreen/sync branch itself is already implemented over for-0.55.0/sync because of compatibility breaking.

This branch also includes the GL_DEPTH_CLAMP disablement when GL_ARB_depth_clamp to avoid spamming the console with warnings when running the game on ATi r300 graphics card, to make debugging easier:

⚠️🚧️ This branch is in very early state! 🚧️⚠️
⚠️🚧️ It's even not ready to review! 🚧️⚠️
⚠️🚧️ This may be full of useless halffloat-to-float-to-halffloat conversions! 🚧️⚠️
⚠️🚧️ I even haven't tested if it doesn't break the usual code path! 🚧️⚠️

I push this branch now because it starts to work and this is so precious I don't want the team to lose that work is something happens to me!

It's now ready to be reviewed, it works for me with both code paths, and is rebased on master.

@illwieckz illwieckz added A-Renderer T-Feature-Request Proposed new feature labels Jun 4, 2024
@illwieckz illwieckz marked this pull request as draft June 4, 2024 03:15
@illwieckz illwieckz changed the title WIP: support graphis cards without ARB_half_float_vertex WIP: support graphics cards without ARB_half_float_vertex Jun 4, 2024
@illwieckz illwieckz force-pushed the illwieckz/no-half-float-vertex branch 2 times, most recently from 08bd535 to e6ec633 Compare June 9, 2024 02:18
@illwieckz illwieckz force-pushed the illwieckz/no-half-float-vertex branch 2 times, most recently from 5cd8db6 to c1d888b Compare June 11, 2024 01:29
@illwieckz
Copy link
Member Author

For unknown reasons, this is broken:

  • mouse pointer
  • minimap
  • console text

@illwieckz illwieckz force-pushed the illwieckz/no-half-float-vertex branch 3 times, most recently from 1898c46 to 38c78e7 Compare June 11, 2024 06:16
@illwieckz
Copy link
Member Author

illwieckz commented Jun 11, 2024

For unknown reasons, this is broken:

  • mouse pointer
  • minimap
  • console text

It was just some stupid copy-paste mistake.

Now it looks like everything is working.

@illwieckz illwieckz force-pushed the illwieckz/no-half-float-vertex branch from b5c0125 to 1b58b39 Compare June 11, 2024 07:04
@illwieckz illwieckz changed the title WIP: support graphics cards without ARB_half_float_vertex Support graphics cards without ARB_half_float_vertex Jun 11, 2024
@illwieckz illwieckz force-pushed the illwieckz/no-half-float-vertex branch from 1b58b39 to 2f8e40f Compare June 11, 2024 07:27
@illwieckz illwieckz marked this pull request as ready for review June 11, 2024 07:27
@illwieckz illwieckz changed the base branch from for-0.55.0/sync to master June 11, 2024 07:28
@illwieckz illwieckz force-pushed the illwieckz/no-half-float-vertex branch from 2f8e40f to 4a01370 Compare June 11, 2024 07:28
@illwieckz
Copy link
Member Author

Now that I verified the branch works, the branch is now targeting master.

It is ready to review.

@illwieckz illwieckz force-pushed the illwieckz/no-half-float-vertex branch 4 times, most recently from 8d8d9f5 to 0d6b114 Compare June 11, 2024 08:05
Copy link
Contributor

@VReaperV VReaperV left a comment

Choose a reason for hiding this comment

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

Same question w. r. t. vector2Copy() vs floatToHalf() usage where the original code only has the former and the branches in this pr seem switched up.

src/engine/client/hunk_allocator.cpp Outdated Show resolved Hide resolved
src/engine/renderer/tr_backend.cpp Outdated Show resolved Hide resolved
src/engine/renderer/tr_surface.cpp Show resolved Hide resolved
src/engine/renderer/tr_vbo.cpp Show resolved Hide resolved
src/engine/renderer/tr_local.h Outdated Show resolved Hide resolved
@illwieckz illwieckz force-pushed the illwieckz/no-half-float-vertex branch from e86ea98 to 7f896cb Compare August 7, 2024 14:49
slipher added a commit that referenced this pull request Aug 9, 2024
Generate the layout for interleaved vertex attribute data at runtime. The
motivation for this is to support OpenGL implementations that don't
provide half float support
(#1179). The vertex "struct"
may contain a 16-bit or 32-bit float, depending on the graphics card.

Now, instead of defining a struct for the data to be uploaded into a
VBO, one must separately specify inputs for each attribute. The input is
defined by a type, base address, stride, etc.; very similarly to the
arguments of glVertexAttribPointer itself. The new version of
R_CreateStaticVBO takes these inputs and writes them to an interleaved
format, performing any neede type conversions along the way.

In this commit just skeletal models (IQM and MD5) are migrated to the
new method.
@slipher
Copy link
Member

slipher commented Aug 9, 2024

You can see my current progress on the branch slipher/runtime-vbo-layout. The dynamically generated vertex attribute layout, which may be configured to use half float or not, is used for IQM, MD3, and MD5 models. It's not ready to test as other VBOs are not ported yet. Remaining steps:

  • Investigate the vertex attribute-related warning spam on station15. IIRC this is caused by some code which creates some verts with the qtangent attribute, then later overwrites that data with the incompatible orientation attribute. I should be careful to avoid breaking this.
  • Use the new vertex attribute layout method for the other static VBOs.
  • Change the dynamic VBO's vertex struct to (always) use float instead of f16_t. This is probably better anyway. Data in the dynamic VBO is only used once and (I believe; will double check) is usually small. Trying to compress it seems pointless. Also tess.verts is sometimes used for data which is not actually sent to the GPU, so avoiding useless conversions should be a win there.

@illwieckz
Copy link
Member Author

Great! I like the way it looks!

As a side note, I prefer the cvar being named r_arb_half_float_vertex (not r_ext_half_float_vertex) as the extension is ARB_half_float_vertex, and ordered alphabetically like the others.

@slipher
Copy link
Member

slipher commented Aug 10, 2024

I spelled out my thoughts on the extension cvar naming. To me it looks like a case of cargo culting gone out of control. Maybe some of the first extensions were EXT ones, which makes some sense as part of the cvar name. But ARB really doesn't.

@illwieckz
Copy link
Member Author

illwieckz commented Oct 1, 2024

I rebased on latest master, which reduced the diff size as some code was just NUKED since.

I added a detection of early RV300 hardware were we should prefer non-half float, and added a workaround cvar that can be disabled to test the r300 driver half-float performance when not using such workaround.

This looks ready to me.

Regarding to the slipher/runtime-vbo-layout branch, I do believe it is the future, but I prefer to see that branch rebased on this one. This one also brings some optimizations and some renaming that makes easier to follow the code and I would like to merge that in all cases anyway.

We can already merge that as it already works, without waiting for slipher/runtime-vbo-layout. While I do believe such deeper redesign is the future, it can wait a bit.

Basically a rebase of slipher/runtime-vbo-layout would have to add an extra parent commit to unsplit the float/half-float if/else code paths. I can help at rebasing it.

@illwieckz illwieckz force-pushed the illwieckz/no-half-float-vertex branch 6 times, most recently from 7e1282e to 3697d53 Compare October 2, 2024 11:43
@slipher
Copy link
Member

slipher commented Oct 7, 2024

I just looked at my vertex layout branch again and it's actually 95% done. The big thing it was waiting on was the autosprite rework (so that we no longer have overlapping attributes). I can have the PR ready in the next day or two.

@illwieckz
Copy link
Member Author

Great!

slipher added a commit that referenced this pull request Oct 9, 2024
Generate the layout for interleaved vertex attribute data at runtime. The
motivation for this is to support OpenGL implementations that don't
provide half float support
(#1179). The vertex "struct"
may contain a 16-bit or 32-bit float, depending on the graphics card.

Now, instead of defining a struct for the data to be uploaded into a
VBO, one must separately specify inputs for each attribute. The input is
defined by a type, base address, stride, etc.; very similarly to the
arguments of glVertexAttribPointer itself. The new version of
R_CreateStaticVBO takes these inputs and writes them to an interleaved
format, performing any neede type conversions along the way.

In this commit just skeletal models (IQM and MD5) are migrated to the
new method.
slipher added a commit that referenced this pull request Oct 9, 2024
Generate the layout for interleaved vertex attribute data at runtime. The
motivation for this is to support OpenGL implementations that don't
provide half float support
(#1179). The vertex "struct"
may contain a 16-bit or 32-bit float, depending on the graphics card.

Now, instead of defining a struct for the data to be uploaded into a
VBO, one must separately specify inputs for each attribute. The input is
defined by a type, base address, stride, etc.; very similarly to the
arguments of glVertexAttribPointer itself. The new version of
R_CreateStaticVBO takes these inputs and writes them to an interleaved
format, performing any neede type conversions along the way.

In this commit just skeletal models (IQM and MD5) are migrated to the
new method.
slipher added a commit that referenced this pull request Oct 10, 2024
Generate the layout for interleaved vertex attribute data at runtime. The
motivation for this is to support OpenGL implementations that don't
provide half float support
(#1179). The vertex "struct"
may contain a 16-bit or 32-bit float, depending on the graphics card.

Now, instead of defining a struct for the data to be uploaded into a
VBO, one must separately specify inputs for each attribute. The input is
defined by a type, base address, stride, etc.; very similarly to the
arguments of glVertexAttribPointer itself. The new version of
R_CreateStaticVBO takes these inputs and writes them to an interleaved
format, performing any neede type conversions along the way.

In this commit just skeletal models (IQM and MD5) are migrated to the
new method.
slipher added a commit that referenced this pull request Oct 10, 2024
Generate the layout for interleaved vertex attribute data at runtime. The
motivation for this is to support OpenGL implementations that don't
provide half float support
(#1179). The vertex "struct"
may contain a 16-bit or 32-bit float, depending on the graphics card.

Now, instead of defining a struct for the data to be uploaded into a
VBO, one must separately specify inputs for each attribute. The input is
defined by a type, base address, stride, etc.; very similarly to the
arguments of glVertexAttribPointer itself. The new version of
R_CreateStaticVBO takes these inputs and writes them to an interleaved
format, performing any neede type conversions along the way.

In this commit just skeletal models (IQM and MD5) are migrated to the
new method.
@illwieckz
Copy link
Member Author

@illwieckz illwieckz closed this Oct 10, 2024
@slipher
Copy link
Member

slipher commented Oct 11, 2024

We still need the commit to add a workaround flipping off half float vertex for the driver with poor quality of implementation.

@slipher slipher reopened this Oct 11, 2024
@illwieckz
Copy link
Member Author

Yes, I'm on it. :-)

@illwieckz
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants