Also, if no VBO is bound bail out of draw_vbo, to avoid trying to
dereference the pointer when filling the shader key.
v2: rename pointer to subcontext to owning_sub (Chia-I)
Signed-off-by: Gert Wollny <gert.wollny@collabora.com>
Reviewed-by: John Bates <jbates@chromium.org>
Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Some paths don't emit rasterizer state or framebuffer state before calling
do_readpixels, and because of that the clamping is not done properly.
Fixes 0d84775435
Fixes several piglit snorm tests.
Signed-off-by: Italo Nicola <italonicola@collabora.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
If the guest is creating texture and the memory
comes from buffer object by creating GL_TEXTURE_BUFFER,
then host creates GL_TEXTURE_BUFFER too.
No texture parameters can be set for GL_TEXTURE_BUFFER.
If there is mismatch between the guest texture format and
the host texture format, for example GL_ALPHA8(guest) and
GL_R8(host), then we can't apply swizzling for such textures.
In such case, add manually swizzling in GLSL shader generation
step.
The logic of this patch:
1. Add additional fields in shader key struct
2. During draw_vbo call check if "manual swizzling" is needed
3. If yes, the add fields in key struct and generate shader again
4. During generation of for example texelFetch instruction
in GLSL put additional instruction for swizzling
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
Telegram (Android App) shows high cpu usage under
virgl. It turns out when the guest send blend_mode
via alpha_src_factor, BLEND_SOFTLIGHT (0x9) would be
treated as PIPE_BLENDFACTOR_SRC1_COLOR (0x9). The
dual_src didn't match dual_src_linked and virgl keeps
linking the same program.
Fix the logic to calculate dual_src to make it as close
as possible to calculate dual_src_linked.
Signed-off-by: Lepton Wu <lepton@chromium.org>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
Fuzzer detected a Use-of-uninitialized-value in p_atomic_dec_zero(),
which uses inline assembly on some platforms. MSAN's documentation claims
unreliable results when instrumenting functions with inline assembly. In
this case, `unsigned char c` is write-only and it's initial value isn't
important, just disable MSAN for p_atomic_dec_zero().
Signed-off-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: John Bates <jbates@chromium.org>
Tipically mesa handles CLAMP_READ_COLOR in gallium frontend, and the
default value for CLAMP_READ_COLOR as defined by the GL spec is
GL_FIXED_ONLY.
As CLAMP_READ_COLOR is really only used for reading pixel color values
from a buffer, there's no problem always returning unclamped colors and
letting the guest mesa clamp it if it wants.
On the other hand, if we clamp it on the host, as we do now, then the
guest mesa only has clamped values to work with, which needless to say,
can't be "unclamped".
Unfortunaly, this fix doesn't work for the GL ES backend, because in
GL ES the colors read with glReadPixels are always clamped.
Fixes piglit's fbo-blending-snorm tests.
Signed-off-by: Italo Nicola <italonicola@collabora.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
p_compiler.h includes stdlib.h which, when compiling against glibc,
transitively includes sys/types.h. The rest of the build relies on this
include in order to provide the uint typedef. Musl's stdlib.h does not
include sys/types.h, causing a missing definition of uint. Include
sys/types.h directly in p_compiler.h.
Signed-off-by: Colin Cross <ccross@android.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
If the OUT[] wasn't .xyzw, then the types wouldn't match and we'd
compile fail. GLSL-to-TGSI always emitted a full vec4 write, but that
shouldn't be required.
Closes: #193
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
This part has been removed from Mesa since commit 3a8a5e77e8f992aaa3539e060885138c2fcddad1
as the patent expired in 2017.
Signed-off-by: Corentin Noël <corentin.noel@collabora.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
If the shader had too many BARRIERs, we'd overflow our preallocated
token buffer and fail to parse the command stream.
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
Probably for some time now the generics are moved from index 0-31
to 9-40, and this doesn't fit the masks we were using, so use
64 bit masks.
Apparently this only triggered an assert and didn't lead to further
failures when the assertion was not enabled so that the CTS didin't
catch this.
We could also use 32 bit masks and shift the index by 9, but if
this would get us into trouble we ever decide to switch the
PIPE_CAP_TGSI_TEXCOORD in mesa/virgl.
Fixes a number of piglits from the glsl-1.10 set that otherwise trigger
an assertion.
Signed-off-by: Gert Wollny <gert.wollny@collabora.com>
Reviewed-by: Rohan Garg <rohan.garg@collabora.com>
Cleanup and make things a little more concise by handling atomic_uint
arrays separately.
Signed-off-by: Rohan Garg <rohan.garg@collabora.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
When emitting GLSL for atomic_uint variables, use the binding location
and offset instead of a generic index as this might lead to cases where
the same variable is bound to two locations with different offsets
causing a linking error on the host.
This fixes:
KHR-GL43.shader_atomic_counters.basic-usage-cs
KHR-GL43.shader_atomic_counters.advanced-usage-switch-programs
Signed-off-by: Rohan Garg <rohan.garg@collabora.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
Introduce two new functions to the decode context for mapping and
unmapping resources. With this we are able to map correctly both GL and
Vulkan resources.
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
It doesn't end up getting used because the sub_ctx->prog check happens
first, but let's not return a surprising truthy value from one fail
path of a bool function.
Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
This crashed for me all the time with nir_to_tgsi triggering compile
failures. The glsl_strings get freed at shader destroy time, no need
to do so here.
Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Refactor only. There are a lot of variables named like *_id/*_index
already.
Signed-off-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
Refactor only. There are a lot of variables named like *_id/*_index
already.
Signed-off-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
Using the 'library' rule instead of 'shared_library' will allow callers
to switch between static and shared library builds using the
default_library config.
It defaults to shared, so the default behavior is the same as before.
Now that vkr_ring supports iov, we can allow larger rings. This also
adjusts VKR_RING_BUFFER_MAX_SIZE to be realistic.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
vkr_ring_read_buffer now supports reading from a ring buffer that is on
top of iov. It is overly complex though.
For further optimization and simplication, we should consider requiring
a logically contiguous virgl_resource. Possible options are requiring a
physically contiguous guest memory (this can have other use cases) or
requiring a host VkDeviceMemory (already doable, but meh).
We also use the chance to replace size_t by uint32_t in
vkr_ring_read_buffer. No functional difference.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
It cannot exceed UINT32_MAX because the ring head and tail are 32-bit.
util_is_power_of_two also truncates to 32-bit.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
It is used to access the extra region, with iov support.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
It is used to access the buffer region. No iov support yet.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
It is used to access the control region, with iov support.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
We want to access its iov directly from vkr_ring.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
They are easier to work with when we support iov_count > 1.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
The only functional difference should be that vkr_region_is_aligned
checks both begin and end.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
I need to work with ring regions in more places, and need helper
functions. vkr_region uses {begin, end}, which seems easier to work
with than {offset, size}.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
It initializes and validates a ring layout, replacing
validate_ring_layout.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
The GLSL spec does not mention a memoryBarrierAtomic function, and the
GLSL to TGSI compiler indicates that a TGSI_MEMBAR_ATOMIC_BUFFER is
created from GLSL's memoryBarrierAtomicCounter.
Ref: 526f7d7790/src/mesa/state_tracker/st_glsl_to_tgsi.cpp (L3737)
This fixes:
KHR-GL43.compute_shader.shared-struct
Signed-off-by: Rohan Garg <rohan.garg@collabora.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
VkDrmFormatModifierPropertiesListEXT::drmFormatModifierCount may be used
uninitialized. It is a codegen bug but a proper fix breaks the
protocol. Until we are ready to finalize the protocol, let's work
around it.
It works so far because Mesa calls the function twice in a row. In the
first call, pDrmFormatModifierProperties is NULL and the uninitialized
value is not used. Instead, it is initialized by the host driver.
In the second call, because of how the temp pool works, the memory gets
reused and the "uninitialized value" is already initialized. Thanks
goes to Yiwei for figuring this out.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
Reviewed-by: Ryan Neph <ryanneph@google.com>
There is also a num_in_clip_dist, so this makes it clearer what is what.
Signed-off-by: Gert Wollny <gert.wollny@collabora.com>
Reviewed-by: Rohan Garg <rohan.garg@collabora.com>
Let the value found in the properties takes preference.
This fixes all compilation and link errors with
KHR-GL43.cull_distance.functional
Signed-off-by: Gert Wollny <gert.wollny@collabora.com>
Reviewed-by: Rohan Garg <rohan.garg@collabora.com>
It seems that the GLSL error messages don't count empty lines, so
it is easier to locate errors when the empty lines are skipped in
the output.
Signed-off-by: Gert Wollny <gert.wollny@collabora.com>
Reviewed-by: Rohan Garg <rohan.garg@collabora.com>
To make the TGSI-GLSL conversion easier to track, keep the original TGSI
text in place when it is to be logged, and print it right before the shader
conversion.
Signed-off-by: Gert Wollny <gert.wollny@collabora.com>
Reviewed-by: Rohan Garg <rohan.garg@collabora.com>
Since we patch the shaders right away only print the shader debug output
after the conversion and the pathcing is finished. This requires that the
tcs pass-through shader printing is moved to the according function.
Signed-off-by: Gert Wollny <gert.wollny@collabora.com>
Reviewed-by: Rohan Garg <rohan.garg@collabora.com>
Commit 603699f1 ("vrend: Cleanup and fix bug in next_sampler_id
tracking") fixed a rendering issue in Factorio but inadvertently changed
the indexing behavior for shadow samplers in some related code cleaning.
shadow_samp_*_locs arrays are currently indexed according to the bit
position of a sampler in samplers_used_mask and shadow_samp_mask, so
sampler_index must be incremented for each sampler, not just for each
shadow sampler.
Fixes: 603699f1 ("vrend: Cleanup and fix bug in next_sampler_id tracking")
Signed-off-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
There is no need to validate string sizes as they are only encoded in
one place, unlike with other dynamic arrays.
We don't validate string sizes when they are non-zero already. This fixes the
decoder to not validate when they are 0.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
A debug message is emitted whenever sampler states are applied and a
NULL state pointer is encountered. NULL pointers indicate that the
default state should be used instead, so there's no need to spam with
error messages approx. 1+ times per draw-call.
commit 405a50d ("vrend: cleanup vrend_bind_sampler_states") adds a more
useful debug message at sampler state bind-time when handle is non-zero
and corresponding state object cannot be located.
Fixes: 32c733f ("src/: replace all instances of "fprintf to stderr" with "vrend_printf"")
Signed-off-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
Discovered while investigating broken floor tile rendering in Factorio
(game). Skipping increment of next_sampler_id, regardless of dirty
status, results in collision with caller's use of next_sampler_id for
additional samplers.
Closes: https://gitlab.freedesktop.org/virgl/virglrenderer/-/issues/227
Fixes: c08c8419 ("vrend: use helper pointers in draw_bind_samplers_shader")
Signed-off-by: Ryan Neph <ryanneph@google.com>
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
Gert Wollny
Italo Nicola
mwezdeck
Lepton Wu
Ryan Neph
Colin Cross
Emma Anholt
Corentin Noël
Rohan Garg
Antonio Caggiano
Dennis Kempin
Chia-I Wu
Yiwei Zhang