James Ross-Gowan authored 3 years ago and Niklas Haas committed 3 years ago

Blit emulation in FL10_1 and below uses a render pass, which means
textures are blittable if they're sampleable and renderable. So this was
a typo...

Some formats don't support this cap, and we don't strictly need it
(since we can work around it using pl_tex_blit_raster).

I plan on perhaps refactoring PL_FMT_CAP_BLITTABLE into
PL_FMT_CAP_BLITTABLE_1D_3D in the future, but for now, this fix is
needed.

And fall back to linking against -latomic otherwise, which seems to be
annoyingly needed on some platforms.

Fixes videolan/libplacebo#181

64-bit is way overkill here and requires the presence of 8-byte atomic
operations, which may even have to be emulated on some platforms.

Arguably even 32-bit is overkill, but should be on the safe side.

Over the years this has failed sporadically on various older GL versions
and I'm at my wit's end trying to keep track of them all. So just ignore
failures here.

Fixes https://github.com/mpv-android/mpv-android/issues/482

Test on a synthetic combination of 8-bit color values and 10-bit sample
depth, and also split the decoding into a separate normalize step to
more closely mimic what happens in the actual renderer.

When e.g. normalizing 10-bits-in-16 color values, the resulting color
values are as they would be for a 16-bit sample, not for a 10-bit
sample. So we need to set the color depth to the sample depth, not the
other way around.

Otherwise, this breaks YCbCr decoding later on, resulting in a green
shift (among other issues).

Should fix https://github.com/mpv-player/mpv/issues/9403

The caps check was wrong, since both caps are needed.

Previously, HLSL registers were allocated based on `pass->descriptors`,
however it's simpler if resource allocation, and hence the arrays saved
with the cached program, only depend on the shader code. This changes
register allocation to use only information provided by SPIRV-Cross'
reflection API instead of `pass->descriptors`.

As a side effect of this change, a few aspects of shader compilation
have been improved and simplified. There is no longer a separate
function to compile GLSL to SPIR-V and SPIR-V to HLSL, since there's no
longer any reason not to do one step right after the other. Also, during
compilation, the resource arrays are now a list of binding numbers
instead of descriptor indexes, which means that the shader cache no
longer depends on the order of the `pass->descriptors` array.

Note that this includes code to support uniform samplerBuffers, however
it's untested since `PL_DESC_BUF_TEXEL_UNIFORM` is only used in
`pl_tex_upload_texel`, which isn't used by d3d11 (yet.)

James Ross-Gowan authored 3 years ago and Niklas Haas committed 3 years ago

The do {} while(0) style of multi-statement macro swallows the
following semicolon (not that it matters here.)

Even for no-op shaders. Failure to do so can lead to shader compiler
errors due to signature mismatch if this is the only pl_shader function
call.

Users can still override this explicitly in practice, and compiling with
-O0 is almost never what you want to do unless you're explicitly
developing the library (in which case, by all means, override it)

By default, with -O0, this uses ~1 MB of stack, which exceeds the
default stack frame size on Windows. There's really no good reason for
us to keep this state on the stack, when static memory would work just
as well.

This is a regression of e85d27f6 introduced in 2b66d74a.

For compatibility with mpv. Requires some minor restructuring of the
code flow to make the aliasing logic conditional, but IMO the code is
better overall for it.

Oops.

Rather than storing the compiler version and name explicitly, just merge
them into the (pre-existing) signature hash. Also, offload the hashing
to the spirv compiler itself.

Make sure struct changes don't suddenly break this pl_mem_hash call.

This reverts commit a3271f80.

This fix was ineffective and, in practice, MSAN already catches the use
of `pl_mem_hash` on unpadded structs for me, e.g.:

==105503==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x509c6c in finalize_pass /home/nand/dev/placebo/build.msan/../src/dispatch.c:867:13
    #1 0x51c2e5 in pl_dispatch_vertex /home/nand/dev/placebo/build.msan/../src/dispatch.c:1492:25
    #2 0x64ecc6 in draw_overlays /home/nand/dev/placebo/build.msan/../src/renderer.c:833:19
    #3 0x61530a in pass_scale_main /home/nand/dev/placebo/build.msan/../src/renderer.c:1749:5
    #4 0x5fa090 in pl_render_image /home/nand/dev/placebo/build.msan/../src/renderer.c:2550:10
    #5 0x4acb0f in pl_render_tests /home/nand/dev/placebo/build.msan/../src/tests/gpu_tests.h:1095:5
    #6 0x4897af in gpu_shader_tests /home/nand/dev/placebo/build.msan/../src/tests/gpu_tests.h:1447:5
    #7 0x487be3 in main /home/nand/dev/placebo/build.msan/../src/tests/vulkan.c:194:9

(At least the one in dispatch.c. It doesn't catch the ones in renderer.c
for some reason, but I'm also less worried about those because they
don't have any relevance outside the renderer's internal state invalidation,
where a false positive doesn't cause real harm)

If the source has multiple pivots for more than one component, the
current code fails compiling, with a macro redefinition error.

This is a safer way to hash structs than calling pl_mem_hash on them,
since it avoids hashing indeterminate values from padding bytes.

Combined into `pl_hash_merge` due to shortcomings of the C macro system
(cannot make this an expression).

This should be bgra8, for consistency with desktop GL. The actual format
is non-opaque and represented using 8 bits. Allows users to use
pl_find_name_fmt(gpu, "bgra8") across all platforms.

Rather than having a separate shader per number of pivots, simply
subdivide into the two cases "no pivots" and "N pivots". The "no pivots"
optimization still covers the (very common) case of there being no
pivots for chroma MMR.

Change the N pivots case from a loop to a tree-style branching select,
which is faster anyways. This helps recover the lost performance (and
then some).

Significantly reduces mid-stream shader recompilation overhead.

In saturation intent, we want to directly stretch from the source
mastering primaries to the target mastering primaries. In either intent,
we want to apply gamut clipping etc. to be relative to the target
mastering primaries, not the nominal encoding gamut.

Change the way this is handled. As an aside, also perform primary
conversion in normalized space, which I believe fixes some sort of
regression in the way gamut mapping is done.

This reverts commit c80ba5d2.

There's no reason for this revert other than that it causes problems
with the CI msan for unfathomable reasons, and I don't have time to
debug it right now. Plus the fact that there's no real motivating case
here anyway.

Sacrifice some of the benefits of the new API (i.e. auto-configuration)
in exchange for more closely mimicking the exact behavior of the older
API.

Tangent to videolan/libplacebo#179

In particular, this fixes an issue where vf_libplacebo didn't strip
mastering metadata even when explicitly converting to SDR, resulting in
a regression where libplacebo effectively didn't actually tone map at
all with the new pl_hdr_metadata-based API.

Fixes videolan/libplacebo#179

This was accidentally the wrong way around. Fix it, and also safeguard
against illegal gamma values.

As reported on IRC by user MoSal

Not that I anticipate us ever having arrays even remotely close to this
in size, but it's the more correct type to use in principle.

In meson.build we had -Wno-pointer-sign, mostly to work around the
otherwise constant warnings about char * vs uint8_t * in string APIs.
However, this obscured a few legitimate bugs, mostly involving int vs
uint32_t in the Vulkan API.

Remove the warning and suppress each usage using explicit casts instead.

To conform to the precedent set forth by the vulkan implementation,
which is to assert explicitly on mis-use and also hold the lock for the
duration of the call.

Good style, since it's decremented from a callback which could be from a
different thread.

These always have to be submitted in lockstep, and doing something like
e.g. resizing the swapchain in the meantime leads to undefined behavior.
Safeguard this with a combination of changes:

1. Hold on to a lock for the duration of the call, thus preventing the
   fbo state from getting invalidated while being rendered to.
2. Make `last_imgidx` contain -1 while not rendering a frame, so we can
   assert on it being set to something valid.

Note that this also fixes a subtle bug where we accidentally passed the
address of `p->last_imgidx` (an int) to pImageIndices (uint32_t *).

James Ross-Gowan authored 3 years ago and Niklas Haas committed 3 years ago

There's no need to hold onto the backbuffer unnecessarily, and it
prevents external code from calling IDXGISwapChain::ResizeBuffers, which
we'd otherwise support just fine.

The assumption that gamut mapping will always be the output of
appropriately tone-mapped content is false. The most obvious
counter-example is when mixing SDR overlays onto HDR content that has
already been tone-mapped. If the overlay has any sort of ringing (e.g.
as a result of scaling), it will contain out-of-range luma values.