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

DarkPlaces only supports one alphaFunc operation, GE128 #267

Merged
merged 1 commit into from Jan 20, 2020

Conversation

@illwieckz
Copy link
Member

illwieckz commented Jan 9, 2020

DarkPlaces only supports one alphaFunc operation:

https://gitlab.com/xonotic/darkplaces/blob/324a5329d33ef90df59e6488abce6433d90ac04c/model_shared.c#L1875-1876

Which is GE128:

https://gitlab.com/xonotic/darkplaces/blob/0ea8f691e05ea968bb8940942197fa627966ff99/render.h#L95

Because of that people may silently introduce regressions in their textures
designed for GT0 by compressing them using a lossy picture format like Jpg.
Xonotic texture known to trigger this bug:

models/desertfactory/textures/shaders/grass01

Using GE128 instead would hide Jpeg artifacts while not breaking that much
non-Darkplaces GT0.

No one operation other than GT0 an GE128 was found in whole Xonotic corpus,
so if there is other operations used in third party maps, they were broken
on Darkplaces and will work there.

Xonotic assets were fixed to make sure any idTech 3 renderers display the Xonotic
textures the same way Darkplaces displays them by telling renderer to use GE128
operator:

But there is thousands of third party maps around there, so the workaround
is required.

Hopefully in case of legit GT0, a GE128 is not that bad, for example with
the grass texture quoted above, it would just be thinner.

Things may be less good in some case, when only the dark frame is expected
to be discarded but there is some dark pixels to keep (typically a road sign).

@illwieckz

This comment has been minimized.

Copy link
Member Author

illwieckz commented Jan 9, 2020

Before:

GT0

After:

GE128

@illwieckz illwieckz added this to To do in Xonotic via automation Jan 9, 2020
@illwieckz illwieckz added the Renderer label Jan 9, 2020
@illwieckz illwieckz force-pushed the illwieckz:dpge128 branch 3 times, most recently from 32b2326 to b0ae494 Jan 9, 2020
@illwieckz

This comment has been minimized.

Copy link
Member Author

illwieckz commented Jan 18, 2020

I'll make this using a dedicated cvar, not the r_dpMaterial one, maybe r_dpBlend, so in the future it would be possible to enable r_dpMaterial without this hack being used (when using assets that are fixed, for example).

…dpBlend

DarkPlaces only supports one alphaFunc operation:

https://gitlab.com/xonotic/darkplaces/blob/324a5329d33ef90df59e6488abce6433d90ac04c/model_shared.c#L1875-1876

Which is GE128:

https://gitlab.com/xonotic/darkplaces/blob/0ea8f691e05ea968bb8940942197fa627966ff99/render.h#L95

Because of that people may silently introduce regressions in their textures
designed for GT0 by compressing them using a lossy picture format like Jpg.
Xonotic texture known to trigger this bug:

    models/desertfactory/textures/shaders/grass01

Using GE128 instead would hide Jpeg artifacts while not breaking that much
non-Darkplaces GT0.

No one operation other than GT0 an GE128 was found in whole Xonotic corpus,
so if there is other operations used in third party maps, they were broken
on Darkplaces and will work there.

Xonotic assets were fixed to make sure any idTech 3 renderers display the Xonotic
textures the same way Darkplaces displays them by telling renderer to use GE128
operator:

- https://gitlab.com/xonotic/xonotic-maps.pk3dir/merge_requests/140
- https://gitlab.com/xonotic/xonotic-data.pk3dir/merge_requests/753
- https://gitlab.com/xonotic/xonotic-nexcompat.pk3dir/merge_requests/2

But there is thousands of third party maps around there, so the workaround
is required.

Hopefully in case of legit GT0, a GE128 is not that bad, for example with
the grass texture quoted above, it would just be thinner.

Things may be less good in some case, when only the dark frame is expected
to be discarded but there is some dark pixels to keep (typically a road sign).
@illwieckz illwieckz force-pushed the illwieckz:dpge128 branch from b0ae494 to b648bc4 Jan 18, 2020
@illwieckz

This comment has been minimized.

Copy link
Member Author

illwieckz commented Jan 18, 2020

I pushed a completely new implementation, introducing r_dpBlend which can be set on and off whatever r_dpMaterial is, and can be switched at runtime (not a latched cvar). This paves the way for games like Xonotic to enable or disable this code path at map load by using some heuristics, because they want to get fully blendFunc support for when maps are ported from other games, or for future maps.

@slipher

This comment has been minimized.

Copy link
Member

slipher commented Jan 20, 2020

LGTM

@illwieckz illwieckz merged commit 14259b3 into DaemonEngine:master Jan 20, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Xonotic automation moved this from To do to Done Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Xonotic
  
Done
2 participants
You can’t perform that action at this time.