27.04.2017, 13:26, "Emil Velikov" <emil.l.veli...@gmail.com>:
> Hi Constantine,
>
> I'm not familiar with r600, so mostly a drive-by idea/nit.

I'm not familiar with graphics :P Well, perhaps a little now.

> On 25 April 2017 at 12:59, Constantine Kharlamov <hi-an...@yandex.ru> wrote:
>
>>  @@ -1746,45 +1747,47 @@ static void evergreen_emit_framebuffer_state(struct 
>> r600_context *rctx, struct r
>>                  radeon_set_context_reg(cs, R_028E50_CB_COLOR8_INFO + (i - 
>> 8) * 0x1C, 0);
>>
>>          /* ZS buffer. */
>>  - if (state->zsbuf) {
>
>>  + if (rctx->framebuffer.dirty_zsbuf) {
>>  + if (state->zsbuf) {
>
> You can fold both if statements in a single one, leaving the
> indentation level as-is.
> As a bonus the resulting patch will be a lot smaller ;-)
>
> Same can be done across the patch.
>
> -Emil

Btw it's how it's done in radeonsi, but I can't do the same in r600g because of 
the drm_minor check. The original code goes like:

        if (state->zsbuf) {
                /* … */
        } else if (rctx->screen->b.info.drm_minor >= 18) {
                /* … */
        }

The only way to keep indentation is to change it like:

        if (rctx->framebuffer.dirty_zsbuf && state->zsbuf) {
                /* … */
        } else if (rctx->framebuffer.dirty_zsbuf && 
rctx->screen->b.info.drm_minor >= 18) {
                /* … */
        }

I think, rather than duplicate the check, it's better to add an indentation 
level.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to