Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ImageBitmap #20650

Open
nox opened this issue Apr 17, 2018 · 20 comments · Fixed by #26009 or #26296
Open

Implement ImageBitmap #20650

nox opened this issue Apr 17, 2018 · 20 comments · Fixed by #26009 or #26296
Labels
A-content/dom Interacting with the DOM from web content

Comments

@nox
Copy link
Contributor

nox commented Apr 17, 2018

https://developer.mozilla.org/en-US/docs/Web/API/ImageBitmap
https://html.spec.whatwg.org/multipage/#imagebitmap

@tigercosmos
Copy link
Contributor

So far any process on this? Where would be the best place to implement it (I mean components, or maybe which file)?

@jdm jdm added the A-content/dom Interacting with the DOM from web content label Apr 28, 2018
@jdm
Copy link
Member

jdm commented Jun 19, 2018

Since this requires implementing a new interface, it would be required to follow the docs (or this guide). This would also require implementing the createImageBitmap API for WindowOrWorkerGlobalScope.

CYBAI added a commit to CYBAI/servo that referenced this issue Sep 1, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 5, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 5, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 6, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 12, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 15, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 15, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 19, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 20, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 20, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 24, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 25, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Sep 25, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Oct 11, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Oct 14, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Oct 14, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Oct 14, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Oct 16, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Oct 17, 2018
CYBAI added a commit to CYBAI/servo that referenced this issue Oct 18, 2018
@pshaughn
Copy link
Contributor

pshaughn commented Dec 6, 2019

In addition to the expected places CYBAI noted, this also has WPT implications in places like html/webappapis/scripting/processingmodel-2/unhandled-promise-rejections, where it is an arbitrarily chosen example of something that returns via promise and has to care about the origin of its input.

@jdm jdm added this to To do in web-platform-test failures via automation Dec 9, 2019
@SasiDharKM
Copy link

I along with two more guys are working on this issue as part of our NCSU Class Project. I understand that the wiki is quite elaborate but we are having trouble understanding what exactly needs to be done. It would be quite helpful if the steps to be taken are further elaborated so as to help us in successfully starting with our implementation. Thanks in advance.

@jdm
Copy link
Member

jdm commented Mar 18, 2020

Could you be more specific about the steps that are unclear right now? What have you understood so far? What have you accomplished?

bors-servo added a commit that referenced this issue Mar 30, 2020
Implementation for ImageBitmap

<!-- Please describe your changes on the following line: -->
Created the boilerplate code for the image bitmap implementation.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #20650

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Mar 30, 2020
Implementation for ImageBitmap

<!-- Please describe your changes on the following line: -->
Created the boilerplate code for the image bitmap implementation.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #20650

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
web-platform-test failures automation moved this from To do to Done Mar 31, 2020
@jdm jdm reopened this Apr 20, 2020
web-platform-test failures automation moved this from Done to In progress Apr 20, 2020
@jdm
Copy link
Member

jdm commented Apr 20, 2020

From personal correspondence:

We started implementing the createImageBitmap method. We were able to do the following steps:
1. Create a new promise object p.
2. Check for option's resizewidth and resizeheight parameters and reject p if they are 0.
3. Check for the usability of the image argument and return p if bad.
4. Created new imageBitmap object.

Now, we are trying to copy the image's bitmap data to imageBitmap object's bitmap_data. Since the image source is canvas, we are not sure how to copy canvas's bitmap_data to imageBitmap's bitmap_data. Could you please provide your input regarding this?

It'd also be helpful if we could get some materials on 
1. how to set the origin-clean flag?
2. how to run steps in parallel?
3. How to resolve a promise object with imageBitmap?

Here is the link to the commit with our initial changes: https://github.com/ramyananth/servo/commit/ed9fdc456056ae1df58cf243cb76e47392a3ff5c

@jdm
Copy link
Member

jdm commented Apr 20, 2020

  1. You will need to add a new boolean member to the ImageBitmap structure that represents the spec's origin-clean flag (and it it needs to be mutated after the ImageBitmap is created, you will need to wrap it in Cell type).
  2. For the purposes of this project, I don't believe we get any benefit from running the steps in parallel, so let's ignore that.
  3. You want the resolve_native method from Promise: https://doc.servo.org/script/dom/promise/struct.Promise.html#method.resolve_native

@JayalakshmiV
Copy link
Contributor

Thank you for the shared information.
Also, we are trying to copy the image's bitmap data to imageBitmap object's bitmap_data. Since the image source is canvas, we are not sure how to copy canvas's bitmap_data to imageBitmap's bitmap_data. Can we add a new member "bitmap_data" in canvas?

@jdm
Copy link
Member

jdm commented Apr 20, 2020

You should use the fetch_image_data method of CanvasState to obtain the pixel data like this code.

@JayalakshmiV
Copy link
Contributor

Hi Josh, we committed our progress and we had a few doubts regarding our work.

We were confused about the following.

  1. To copy the image's bitmap data, we were supposed to use the fetch_image_data method from canvas_state.rs.
    However, that method is private. We are not sure how we are supposed to use this method. Are we allowed to make changes to the canvas_state.rs file in this case?
  2. We have let the origin_clean flag be mutated inside the same match function that we wrote to check if the image source is valid.
    Can we do that, or are we supposed to create another match function?
  3. Are we also supposed to add the origin_clean attribute to the WebIDL and hence create getter method for the origin_clean attribute as well?

Thank you in advance for your help.

@jdm
Copy link
Member

jdm commented Apr 22, 2020

  1. Make any changes to CanvasState that you need!
  2. Doing it inside the existing match sounds fine.
  3. The WebIDL contents come directly from the specification, so there is no need to modify it. It represents the interface that is accessible to web content via JavaScript, and the origin-clean flag is not part of that.

@ramyananth
Copy link

Hi Josh, we pushed some more changes to the codes and have a couple of build issues that we are facing,

  1. How do we use enum into fetching all the canvas data elements. We used this link to make store the image_data and image_size but, fetch_image_data didn't seem to exist for canvas element. So, we switched to fetch_all_data on this link. How do you suggest we proceed? I've attached the error screenshot here.
    enumError
  2. How do we access the structure attributes for the imageBitmap (for bitmap_data and origin_clean). We run into the "no field" error while using the dot operator?

Thanks in advance for your help.

@jdm
Copy link
Member

jdm commented Apr 23, 2020

@ramyananth If you look closely at the code, you will see that fetch_all_data returns an Option type, like the error message suggests. The reason that the code in fetch_all_data works is that it uses the ? operator to immediately return a None value when the return value is None, otherwise it extracts the contents of the Some and continues executing (see the std::option docs for more about Option types).

Rather than attaching screenshots of errors, in the future please copy and paste the text from the terminal. It is easier for me to read that way.

As for your second question, I think you will need to provide more information to help me diagnose what's going wrong. In general, for a struct S that has field F, the dot operator will allow you to access that field. Since that is not working, you presumably are either trying to access a non-public field of ImageBitmap from a file that is not imagebitmap.rs, or the field you are trying to access does not actually exist, or you are trying to access the field on a value that is not actually a ImageBitmap value.

For further help diagnosing it, please push your code to a branch and link the line of code that is triggering the error, and provide the full error message that you are receiving.

bors-servo added a commit that referenced this issue Apr 25, 2020
WIP: Implementing createImageBitmap

<!-- Please describe your changes on the following line: -->
Implementing createImageBitmap method for canvas image source.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #20650 (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
WIP: Implementing createImageBitmap

<!-- Please describe your changes on the following line: -->
Implementing createImageBitmap method for canvas image source.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #20650 (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
Implementing createImageBitmap

<!-- Please describe your changes on the following line: -->
Implementing createImageBitmap method for canvas image source.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #20650 (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
Implementing createImageBitmap

<!-- Please describe your changes on the following line: -->
Implementing createImageBitmap method for canvas image source.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #20650 (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 30, 2020
Implementing createImageBitmap

<!-- Please describe your changes on the following line: -->
Implementing createImageBitmap method for canvas image source.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #20650 (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
web-platform-test failures automation moved this from In progress to Done Apr 30, 2020
@nradhak2
Copy link

nradhak2 commented May 1, 2020

Hi Josh,

We just had a few doubts regarding testing our functionalities manually. You had suggested having a HTML <script> to test it. Going through some material on this, we understood that this could be possible using the HTML!() macro within the RUST code and embedding the HTML script within it. Are we allowed to modify the RUST code with our code to manually test the functionalities or is there another way to do so? Any additional material on this matter would be really helpful. Thank you in advance for your help.

@jdm
Copy link
Member

jdm commented May 1, 2020

No, you would need to create a real HTML file and run ./mach run path/to/file.html to load it in Servo.

@jdm jdm reopened this May 1, 2020
web-platform-test failures automation moved this from Done to In progress May 1, 2020
@jdm jdm moved this from In progress to To do in web-platform-test failures Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-content/dom Interacting with the DOM from web content
Projects
Development

Successfully merging a pull request may close this issue.

9 participants