Do not use lookup tables for put_image_data operations #9599

michaelwu · 2016-02-11T01:31:29Z

#8086 is pretty sketchy. If floating point math is a bottleneck here, then we can switch to fixed point math. Using a 64kbyte lookup table to avoid floating point multiplication isn't worth it. For really fast CPU results, use integer SIMD instructions to handle more than one pixel at a time.

let alpha = imagedata[src_offset + 3] as u16;
dest.push((imagedata[src_offset + 2] as u16 * alpha / 256) as u8);
// etc

128 can be added before dividing to round more accurately.

cc @glennw @jdm

magni- · 2016-03-07T09:08:02Z

I'd like to claim this one if it's still free

jdm · 2016-03-07T14:02:53Z

Go for it!


        Auto merge of #9911 - magni-:dont_use_lookup_tables, r=<try>

@michaelwu

Don't use lookup tables for put_image_data and GetImageData Fixes #9599 "This is the first Rust code I have ever written" (and also my first time writing real browser code, it's been quite the learning experience). Some questions: > For really fast CPU results, use integer SIMD instructions to handle more than one pixel at a time. This was out of the scope of #9599, right? I started looking into doing that, but it seems to be a lot more work than the `E-easy` label would suggest. [`std::simd`](https://doc.rust-lang.org/1.0.0/std/simd/index.html) is marked as "unstable", is that a blocker? > 128 can be added before dividing to round more accurately. @michaelwu, what did you mean by that? Also, the #9599 is `Do not use lookup tables for {Get,Put}ImageData operations`, but we only use lookup tables for the `Put`, not the `Get`, right? Sorry for all the noobish questions.  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9911)


        Auto merge of #9911 - magni-:dont_use_lookup_tables, r=<try>

@michaelwu

Don't use lookup tables for put_image_data and GetImageData Fixes #9599 "This is the first Rust code I have ever written" (and also my first time writing real browser code, it's been quite the learning experience). Some questions: > For really fast CPU results, use integer SIMD instructions to handle more than one pixel at a time. This was out of the scope of #9599, right? I started looking into doing that, but it seems to be a lot more work than the `E-easy` label would suggest. [`std::simd`](https://doc.rust-lang.org/1.0.0/std/simd/index.html) is marked as "unstable", is that a blocker? > 128 can be added before dividing to round more accurately. @michaelwu, what did you mean by that? Also, the #9599 is `Do not use lookup tables for {Get,Put}ImageData operations`, but we only use lookup tables for the `Put`, not the `Get`, right? Sorry for all the noobish questions.  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9911)


        Auto merge of #9911 - magni-:dont_use_lookup_tables, r=<try>

@michaelwu

Don't use lookup tables for put_image_data and GetImageData Fixes #9599 "This is the first Rust code I have ever written" (and also my first time writing real browser code, it's been quite the learning experience). Some questions: > For really fast CPU results, use integer SIMD instructions to handle more than one pixel at a time. This was out of the scope of #9599, right? I started looking into doing that, but it seems to be a lot more work than the `E-easy` label would suggest. [`std::simd`](https://doc.rust-lang.org/1.0.0/std/simd/index.html) is marked as "unstable", is that a blocker? > 128 can be added before dividing to round more accurately. @michaelwu, what did you mean by that? Also, the #9599 is `Do not use lookup tables for {Get,Put}ImageData operations`, but we only use lookup tables for the `Put`, not the `Get`, right? Sorry for all the noobish questions.  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9911)

michaelwu · 2016-03-14T20:47:17Z

Renamed this because unpremultiplying for GetImageData is a harder problem.


        Auto merge of #9911 - magni-:dont_use_lookup_tables, r=<try>

@michaelwu

Do not use lookup tables for put_image_data Fixes #9599 "This is the first Rust code I have ever written" (and also my first time writing real browser code, it's been quite the learning experience). Some questions: > For really fast CPU results, use integer SIMD instructions to handle more than one pixel at a time. This was out of the scope of #9599, right? I started looking into doing that, but it seems to be a lot more work than the `E-easy` label would suggest. [`std::simd`](https://doc.rust-lang.org/1.0.0/std/simd/index.html) is marked as "unstable", is that a blocker? > 128 can be added before dividing to round more accurately. @michaelwu, what did you mean by that? Also, the #9599 is `Do not use lookup tables for {Get,Put}ImageData operations`, but we only use lookup tables for the `Put`, not the `Get`, right? Sorry for all the noobish questions.  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9911)


        Auto merge of #9911 - magni-:dont_use_lookup_tables, r=<try>

@michaelwu

Do not use lookup tables for put_image_data Fixes #9599 "This is the first Rust code I have ever written" (and also my first time writing real browser code, it's been quite the learning experience). Some questions: > For really fast CPU results, use integer SIMD instructions to handle more than one pixel at a time. This was out of the scope of #9599, right? I started looking into doing that, but it seems to be a lot more work than the `E-easy` label would suggest. [`std::simd`](https://doc.rust-lang.org/1.0.0/std/simd/index.html) is marked as "unstable", is that a blocker? > 128 can be added before dividing to round more accurately. @michaelwu, what did you mean by that? Also, the #9599 is `Do not use lookup tables for {Get,Put}ImageData operations`, but we only use lookup tables for the `Put`, not the `Get`, right? Sorry for all the noobish questions.  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9911)


        Auto merge of #9911 - magni-:dont_use_lookup_tables, r=<try>

@michaelwu

Do not use lookup tables for put_image_data Fixes #9599 "This is the first Rust code I have ever written" (and also my first time writing real browser code, it's been quite the learning experience). Some questions: > For really fast CPU results, use integer SIMD instructions to handle more than one pixel at a time. This was out of the scope of #9599, right? I started looking into doing that, but it seems to be a lot more work than the `E-easy` label would suggest. [`std::simd`](https://doc.rust-lang.org/1.0.0/std/simd/index.html) is marked as "unstable", is that a blocker? > 128 can be added before dividing to round more accurately. @michaelwu, what did you mean by that? Also, the #9599 is `Do not use lookup tables for {Get,Put}ImageData operations`, but we only use lookup tables for the `Put`, not the `Get`, right? Sorry for all the noobish questions.  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9911)


        Auto merge of #9911 - magni-:dont_use_lookup_tables, r=michaelwu

@michaelwu

Do not use lookup tables for put_image_data Fixes #9599 "This is the first Rust code I have ever written" (and also my first time writing real browser code, it's been quite the learning experience). Some questions: > For really fast CPU results, use integer SIMD instructions to handle more than one pixel at a time. This was out of the scope of #9599, right? I started looking into doing that, but it seems to be a lot more work than the `E-easy` label would suggest. [`std::simd`](https://doc.rust-lang.org/1.0.0/std/simd/index.html) is marked as "unstable", is that a blocker? > 128 can be added before dividing to round more accurately. @michaelwu, what did you mean by that? Also, the #9599 is `Do not use lookup tables for {Get,Put}ImageData operations`, but we only use lookup tables for the `Put`, not the `Get`, right? Sorry for all the noobish questions.  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9911)


        Auto merge of #9911 - magni-:dont_use_lookup_tables, r=michaelwu

@michaelwu

Do not use lookup tables for put_image_data Fixes #9599 "This is the first Rust code I have ever written" (and also my first time writing real browser code, it's been quite the learning experience). Some questions: > For really fast CPU results, use integer SIMD instructions to handle more than one pixel at a time. This was out of the scope of #9599, right? I started looking into doing that, but it seems to be a lot more work than the `E-easy` label would suggest. [`std::simd`](https://doc.rust-lang.org/1.0.0/std/simd/index.html) is marked as "unstable", is that a blocker? > 128 can be added before dividing to round more accurately. @michaelwu, what did you mean by that? Also, the #9599 is `Do not use lookup tables for {Get,Put}ImageData operations`, but we only use lookup tables for the `Put`, not the `Get`, right? Sorry for all the noobish questions.  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9911)

jdm added the A-content/canvas label Feb 11, 2016

michaelwu added E-easy I-perf-slow labels Mar 2, 2016

jdm added the C-assigned label Mar 7, 2016

magni- mentioned this issue Mar 8, 2016

Do not use lookup tables for put_image_data #9911

Merged

michaelwu changed the title ~~Do not use lookup tables for {Get,Put}ImageData operations~~ Do not use lookup tables for put_image_data operations Mar 14, 2016

jdm added the C-has open PR label Mar 30, 2016

bors-servo closed this in #9911 Apr 1, 2016

servo / servo

Do not use lookup tables for put_image_data operations #9599

Do not use lookup tables for put_image_data operations #9599

michaelwu commented Feb 11, 2016

magni- commented Mar 7, 2016

jdm commented Mar 7, 2016

michaelwu commented Mar 14, 2016

servo / servo

Join GitHub today

Do not use lookup tables for put_image_data operations #9599

Do not use lookup tables for put_image_data operations #9599

Comments

michaelwu commented Feb 11, 2016

magni- commented Mar 7, 2016

jdm commented Mar 7, 2016

michaelwu commented Mar 14, 2016