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

Support building for Magic Leap. #21985

Merged
merged 1 commit into from Oct 29, 2018

Conversation

Projects
None yet
7 participants
@asajeffrey
Copy link
Member

asajeffrey commented Oct 19, 2018

This PR gets Servo to build for Magic Leap, and provides a dummy Servo2D app.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because it's a port to a new architecture

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

highfive commented Oct 19, 2018

Heads up! This PR modifies the following files:

@highfive

This comment has been minimized.

Copy link

highfive commented Oct 19, 2018

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify gfx and script code, but no tests are modified. Please consider adding a test!
@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Oct 19, 2018

It depends on servo/mozjs#158 and servo/media#158.

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Oct 22, 2018

Filed #21999 to get servo-media updated.

@jdm jdm assigned jdm and unassigned emilio Oct 22, 2018

libservo = { path = "../../components/servo" }
servo-egl = "0.2"
log = "0.4"
smallvec = "0.6"

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

nit: let's start this in alphabetic order at least.

screen_avail: TypedSize2D::new(500, 500),
window: (TypedSize2D::new(500, 500), TypedPoint2D::new(0, 0)),
framebuffer: TypedSize2D::new(500, 500),
viewport: TypedRect::new(TypedPoint2D::new(0, 0), TypedSize2D::new(500, 500)),

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

nit: weird indentation.

fn get_coordinates(&self) -> EmbedderCoordinates {
EmbedderCoordinates {
hidpi_factor: TypedScale::new(1.0),
screen: TypedSize2D::new(500, 500),

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

Shall we move these magic numbers into constants at least?

Resource::QuirksModeCSS => &include_bytes!("../../../resources/quirks-mode.css")[..],
Resource::RippyPNG => &include_bytes!("../../../resources/rippy.png")[..],
Resource::DomainList => &include_bytes!("../../../resources/public_domains.txt")[..],
Resource::BluetoothBlocklist => &include_bytes!("../../../resources/gatt_blocklist.txt")[..],

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

Is it worth adding a build script to this crate that forces a rebuild if any of these files are modified?


impl log::Log for MLLogger {
fn enabled(&self, metadata: &log::Metadata) -> bool {
metadata.level() <= log::Level::Info

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

Maybe store the level in a constant so the set_max_level call earlier matches?


// The prism dimensions
const glm::vec3 Servo2D::getInitialPrismExtents() const {
return glm::vec3(0.5f, 0.5f, 0.5f);

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

Do these relate to the 500s in libmlservo, or is that just coincidence?

lumin::ui::Cursor::SetScale(prism_, 0.03f);
instanceInitialScenes();

// Get the panar resource that holds the EGL context

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

panar?

EGLSurface surf = plane_->getEGLSurface();
EGLDisplay dpy = eglGetDisplay(EGL_DEFAULT_DISPLAY);
eglMakeCurrent(dpy, surf, surf, ctx);
glViewport(0, 0, 500, 500);

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

Does this need to match the 500s in libmlservo? Can we pass this value in to Rust?

int main(int argc, char **argv)
{
ML_LOG(Debug, "Servo2D Starting.");
Servo2D myApp;

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

Does argv contain meaningful values for us? Can we accept a URL on the command line?

EGLSurface surf = plane_->getEGLSurface();
EGLDisplay dpy = eglGetDisplay(EGL_DEFAULT_DISPLAY);
eglMakeCurrent(dpy, surf, surf, ctx);
glViewport(0, 0, 500, 500);

This comment has been minimized.

@jdm

jdm Oct 22, 2018

Member

Let's make this a named constant somewhere.

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Oct 23, 2018

☔️ The latest upstream changes (presumably #20755) made this pull request unmergeable. Please resolve the merge conflicts.

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Oct 23, 2018

I did some fixes, but can't test the C++ code because I'm at home rather than in the office.

@asajeffrey asajeffrey force-pushed the asajeffrey:magicleap branch from e261c62 to 5aa2dc5 Oct 23, 2018

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Oct 23, 2018

Rebased.

@asajeffrey asajeffrey changed the title [WIP] Support building for Magic Leap. Support building for Magic Leap. Oct 23, 2018

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Oct 23, 2018

Updated mozjs_sys which was the last external blocker.

@paulrouget

This comment has been minimized.

Copy link
Contributor

paulrouget commented Oct 24, 2018

I see that you introduced a new port. Did you consider using the c api of libsimpleservo instead of libmlservo?

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Oct 24, 2018

@paulrouget yes, originally I was using libsimpleservo but ran into horrible linking problems which went away if I used libservo directly. My suspicion is that the ML is going to be a different enough target from Android that we're going to need a different port at some point anyway.

@paulrouget

This comment has been minimized.

Copy link
Contributor

paulrouget commented Oct 24, 2018

libsimpleservo can be compiled without any android stuff (it compiles on Desktop for example). It's supposed to be capable of being dynamically loaded like libservo. I'd be interested to know what kind of issues you ran into.

I don't want to block this, but maybe later we could look at how to re-use libsimpleservo for ML.

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Oct 24, 2018

It's been a while, but there was some issue about libsimpleservo being dynamically loaded causing symbols not to be found. I may have fixed that in the mean time, e.g. by statically loading openssl.


## Building the Servo2D application

From inside the `ports/magicleap/Servo2D` directory:

This comment has been minimized.

@ferjm

ferjm Oct 29, 2018

Member

s/ports/support

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Oct 29, 2018

@bors-servo delegate+
I'm fine with keeping the question about the URL as a followup.

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Oct 29, 2018

✌️ @asajeffrey can now approve this pull request

@asajeffrey asajeffrey force-pushed the asajeffrey:magicleap branch from ab2e653 to dab8f4a Oct 29, 2018

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Oct 29, 2018

@bors-servo r=jdm

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Oct 29, 2018

📌 Commit dab8f4a has been approved by jdm

bors-servo added a commit that referenced this pull request Oct 29, 2018

Auto merge of #21985 - asajeffrey:magicleap, r=jdm
Support building for Magic Leap.

<!-- Please describe your changes on the following line: -->

This PR gets Servo to build for Magic Leap, and provides a dummy Servo2D app.

---
<!-- 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
- [X] These changes do not require tests because it's a port to a new architecture

<!-- 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. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21985)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Oct 29, 2018

⌛️ Testing commit dab8f4a with merge 0ec0f70...

@bors-servo

This comment has been minimized.

@bors-servo bors-servo merged commit dab8f4a into servo:master Oct 29, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@asajeffrey asajeffrey referenced this pull request Oct 29, 2018

Closed

Magic Leap minimum viable demo #22043

5 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.