Re: [vdagent-win PATCH v12 3/6] Write code to decode PNG format

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

 




On 26 Jul 2017, at 09:58, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:


On 25 Jul 2017, at 20:01, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:


On Mon, Jul 24, 2017 at 01:39:35PM +0100, Frediano Ziglio wrote:
+size_t PngCoder::convert_to_dib(uint8_t *out_buf, const uint8_t *data,
size_t size)
+{
[...]
+    const unsigned int width = png_get_image_width(png, info);
+    const unsigned int height = png_get_image_height(png, info);
+    const size_t stride = compute_dib_stride(width, out_bits);
+    const size_t image_size = stride * height;
+    const int palette_colors = [=]() {

If you want to use a function, then give it a name, if you don't want to
use a function, then this can just go in convert_to_dib() body rather
than through a lambda (I know the latter means removing the 'const’).

Removing the ‘const’? Why?


Lambdas are functions.

I believe Christophe is objecting to using a lambda whereas all other
computations use named functions. But frankly, here, I would have
written:

    const int palette_colors =
          out_bits > 8  ? 0
        : bits == 2     ? 4
        : 1 << out_bits;
This was exactly the previous version, just my version has some additional comments
to make more readable.

So what made you change it? Do you feel that the lambda is more readable somehow?
Christophe said was not really readable and I used lambda as I feel was more readable.
Not that I don’t like lambdas, but here, it’s a lot of additional syntactic noise.

Out of curiosity, I submitted both variants of the code to g++, and got
exactly the same generated code.

I personally think the purpose of that code was clear even without
a name.
Using a "standard" function requires to declare parameters and pass
them manually, something that lambda capture can do automatically.


Christophe


By the way, got new version with more old style code.

Frediano

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]