Siarhei Siamashka <siarhei.siamas...@gmail.com> writes: > > 15 files changed, 743 insertions(+), 470 deletions(-) > > On the other hand, I'm not so happy about the implementation. The number of > lines of code as well as the complexity has increased, and the performance > seems to have dropped a little bit in some of the cases.
The performance numbers you posted seem very close to noise to me; in some cases the new code is faster. Considering that this was measured with the CPU specific fast paths turned off, most users won't actually see even this small slowdown. > I really want to see some practical benefits which depend on these > added iterators in the near future. It isn't clear to me what specifically you are suggesting. One practical benefit of the patch set is that it fixes the bug where destination properties aren't ignored as they should be. This bug could probably be fixed some other way, but it doesn't seem to be possible with separating source- and destination accessors in some way. This separation is where most of the additional new lines are coming from. In fact, as far as I can tell, the iterators make the code _less_ complex. Compare the new loop in pixman-general.c: for (i = 0; i < height; ++i) { uint32_t *s, *m, *d; m = mask_iter.get_scanline (&mask_iter, NULL); s = src_iter.get_scanline (&src_iter, m); d = dest_iter.get_scanline (&dest_iter, NULL); compose (imp->toplevel, op, d, s, m, width); dest_iter.write_back (&dest_iter); } with the old one: for (i = 0; i < height; ++i) { /* fill first half of scanline with source */ if (fetch_src) { if (fetch_mask) { /* fetch mask before source so that fetching of source can be optimized */ fetch_mask (mask, mask_x, mask_y + i, width, (void *)mask_buffer, 0); if (mask_class == SOURCE_IMAGE_CLASS_HORIZONTAL) fetch_mask = NULL; } if (src_class == SOURCE_IMAGE_CLASS_HORIZONTAL) { fetch_src (src, src_x, src_y + i, width, (void *)src_buffer, 0); fetch_src = NULL; } else { fetch_src (src, src_x, src_y + i, width, (void *)src_buffer, (void *)mask_buffer); } } else if (fetch_mask) { fetch_mask (mask, mask_x, mask_y + i, width, (void *)mask_buffer, 0); } if (store) { /* fill dest into second half of scanline */ if (fetch_dest) { fetch_dest (dest, dest_x, dest_y + i, width, (void *)dest_buffer, 0); } /* blend */ compose (imp->toplevel, op, (void *)dest_buffer, (void *)src_buffer, (void *)mask_buffer, width); /* write back */ store (&(dest->bits), dest_x, dest_y + i, width, (void *)dest_buffer); } else { /* blend */ compose (imp->toplevel, op, bits + (dest_y + i) * stride + dest_x, (void *)src_buffer, (void *)mask_buffer, width); } } Soren _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman