[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [E-devel] patch for edje



ZigsMcKenzie [2006-06-11 20:50]:

Some comments.

> +	STATE_COLOR_CLASS = 10,
> +	STATE_REL1 = 11,
> +    STATE_REL1_TO = 12,

Obviously bad indentation here (and in some other places).

> +      case EDJE_STATE_PARAM_COLOR_CLASS:
> +         CHKPARAM(3);
> +
> +	 char *cc;

Declaring variables in the middle of a block isn't allowed in C89, and
IMO we shouldn't break C89 support for no good reason
(same problem in other places, too).

> +      case EDJE_STATE_PARAM_REL1_TO:
> +	 CHKPARAM(4);
> +
> +         GETINT(rp->custom.description->rel1.id_x, params[3]);
> +         GETINT(rp->custom.description->rel1.id_y, params[4]);
> +
> +	 if (rp->param1.description->rel1.id_x >= 0)
> +	   rp->param1.rel1_to_x = ed->table_parts[rp->param1.description->rel1.id_x % ed->table_parts_size];
> +	 if (rp->param1.description->rel1.id_y >= 0)
> +	   rp->param1.rel1_to_y = ed->table_parts[rp->param1.description->rel1.id_y % ed->table_parts_size];

Bad indentation here.

> +	 GETINT(rp->custom.description->visible, params[3]);
> +
> +	 break;	    
>        default:
>  	 break;
>       }
>  
> +   ed->dirty=1;
> +   _edje_recalc(ed);
> +
>     return 0;
>  }

OY! Always recalcing the Edje here seems too excessive. It used to work
well without it for the ops we had before your patch AFAIK, so you
should prolly only recalc for the new ops, and where it's really
necessary.

Regards,
Tilman

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Attachment: pgpaUwGcox9Ve.pgp
Description: PGP signature