On 2016-03-25 22:20, Ilia Mirkin wrote:
On Mar 25, 2016 4:43 AM, <eocallag...@alterapraxis.com> wrote:
On 2016-03-25 14:02, Ilia Mirkin wrote:
On Thu, Mar 24, 2016 at 8:11 PM, Edward O'Callaghan
<eocallag...@alterapraxis.com> wrote:
Using PIPE_FORMAT_NONE to indicate what MSAA modes are supported
with a framebuffer using no attachment.
Signed-off-by: Edward O'Callaghan <eocallag...@alterapraxis.com>
---
src/mesa/state_tracker/st_atom_framebuffer.c | 51
++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/src/mesa/state_tracker/st_atom_framebuffer.c
b/src/mesa/state_tracker/st_atom_framebuffer.c
index ae883a2..07854ca 100644
--- a/src/mesa/state_tracker/st_atom_framebuffer.c
+++ b/src/mesa/state_tracker/st_atom_framebuffer.c
@@ -64,6 +64,44 @@ update_framebuffer_size(struct
pipe_framebuffer_state *framebuffer,
framebuffer->height = MIN2(framebuffer->height,
surface->height);
}
+/**
+ * Round up the requested multisample count to the next supported
sample size.
+ */
+static unsigned
+framebuffer_quantize_num_samples(struct pipe_screen *screen,
unsigned num_samples)
+{
+ int quantized_samples = 0;
+ bool msaa_mode_supported;
+
+ if (!num_samples)
+ return 0;
+
+ assert(screen);
+
+ /* Assumes the highest supported MSAA is x32 on any hardware
*/
+ for (unsigned msaa_mode = 32; msaa_mode >= 1; msaa_mode =
msaa_mode/2) {
This should probably start at MaxFramebufferSamples right? Also
msaa_mode >= num_samples? [then you can get rid of the if below]
I did it in this manner because I don't trust all C compilers to
warn sufficiently
on `num_samples' overflows turning this into a infinite loop even
though it is a
unsigned type. The micro-optimization serves no purpose because the
optimizer will
trivially reduce the loop down, not that it has that many iterations
any way. The
loop as-is is both well bounded and deterministic, nice qualities to
have.
"Premature optimization is the root of all evil" ~ Donald Knuth's.
I was going for clarity and simplicity, not runtime efficiency. Fewer
lines of code to read, fewer conditions. For loop semantics are fairly
well defined, compilers tend to get those things right.
I was not referring to loop semantics or if a compiler can understand
how
to lower a loop correctly, you didn't really read what I said. Point is,
parameterizing the loop with a function argument to save one line of
code
while losing some safety and determinism does hardly anything to make a
argument for this to be changed imho. I much prefer how it is now, a
simple
constant deterministic loop, very clear.
And lastly I don't know if it's a valid assumption that we can
always
just divide by 2. That said, I don't know of any hw that actually
supports non-power-of-two MSAA levels, so perhaps it's OK.
You are way over engineering here; it is a totally reasonable
assumption and if such
hardware does exist which we would support (I can`t see any in the
tree currently as
far as I am aware) then they can provide follow up fixes.
I'm flexible on this one... If no one else cares, I don't care either.
+ assert(!(msaa_mode > 32 || msaa_mode == 0)); /* be safe
from int overflows */
+ if (msaa_mode >= num_samples) {
+ /**
+ * For ARB_framebuffer_no_attachment, A format of
+ * PIPE_FORMAT_NONE implies what number of samples is
+ * supported for a framebuffer with no attachment. Thus
the
+ * drivers callback must be adjusted for this.
+ */
+ msaa_mode_supported =
screen->is_format_supported(screen,
+ PIPE_FORMAT_NONE,
PIPE_TEXTURE_2D,
+ msaa_mode,
PIPE_BIND_RENDER_TARGET);
+ /**
+ * Check if the MSAA mode that is higher than the
requested
+ * num_samples is supported, and if so returning it.
+ */
+ if (msaa_mode_supported)
+ quantized_samples = msaa_mode;
+ }
+ }
+
+ return quantized_samples;
+}
/**
* Update framebuffer state (color, depth, stencil, etc. buffers)
@@ -72,6 +110,8 @@ static void
update_framebuffer_state( struct st_context *st )
{
struct pipe_framebuffer_state *framebuffer =
&st->state.framebuffer;
+ struct pipe_context *pipe = st->pipe;
+ struct pipe_screen *screen = pipe->screen;
struct gl_framebuffer *fb = st->ctx->DrawBuffer;
struct st_renderbuffer *strb;
GLuint i;
@@ -82,6 +122,17 @@ update_framebuffer_state( struct st_context
*st )
framebuffer->width = UINT_MAX;
framebuffer->height = UINT_MAX;
+ /**
+ * Quantize the derived default number of samples:
+ *
+ * A query to the driver of supported MSAA values the
+ * hardware supports is done as to legalize the number
+ * of application requested samples, NumSamples.
+ * See commit eb9cf3c for more information.
+ */
+ fb->DefaultGeometry._NumSamples =
+ framebuffer_quantize_num_samples(screen,
fb->DefaultGeometry.NumSamples);
+
/*printf("------ fb size %d x %d\n", fb->Width, fb->Height);*/
/* Examine Mesa's ctx->DrawBuffer->_ColorDrawBuffers state
--
2.5.5
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev [1]
Links:
------
[1] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
