Make it possible to position the viewport anywhere

[paulrouget]

Requires: servo/servo#17091

---

This change is 

[paulrouget]

I just change the viewport coordinate in device.rs. Is that enough?

[kvark]

I find the new scheme confusing:

• SetViewGeometry doesn't exactly have any relation to geometry, does it? What makes the old name SetWindowParameters worse?
• RenderBackend has rendering_rect and viewport_rect, but the actual glViewport call is done with rendering_rect.origin...

[paulrouget]

SetViewGeometry doesn't exactly have any relation to geometry, does it?

Maybe geometry is not the right word (in french it is also used to describe a "layout" of a room).

What makes the old name SetWindowParameters worse?

The problem is "window". From the embedder perspective, a window could be native window, or the page window.

RenderBackend has rendering_rect and viewport_rect, but the actual glViewport call is done with rendering_rect.origin...

Yeah it's because the web viewport of the gl viewport are not the same thing :(

Definitively confusing.

Maybe:

struct ViewParameter {
  inner_rect: … // CSS viewport. Top-left corner of this rect is 0x0 CSS pixel
  outer_rect: … // Where content is actually clipped

  //  Will eventually migrate to the a future CompositorParameter struct:
  chrome_rect: … // Chrome window dimensions. Used from window.moveTo/innerHeight/outerWidth etc.
  hidpi_factor: …
}

Any other ideas are welcome :)

[kvark]

Trying to evolve your proposal further:

struct Viewport {
  content_rect: … // CSS viewport. Top-left corner of this rect is 0x0 CSS pixel
  outer_rect: … // Where content is actually clipped

  chrome_rect: … 
  hidpi_factor: …
}

[paulrouget]

I don't want to bikeshed too much, but I'd rather avoid "Viewport". Like Window, it has multiple meanings (gl viewport and css viewport being 2 different things). From the embedder point of view, we usually use the term "View".

This struct will be used to initialize a View, but also update the View layout. So I don't want to just use use struct View but struct ViewParameters if that makes sense?

In the future, we will also add overscroll parameters (see #1314).

#[derive(Copy, Clone)]
pub struct ViewParameters {
    /// CSS viewport. The inner rect top left corner is the 0x0 CSS pixel (before scroll).
    pub inner_rect: TypedRect<u32, DevicePixel>,
    /// The outer rect is where content gets clipped. The outer rect
    /// is equal to or larger than the viewport area. This makes it possible to
    /// draw content outside of the CSS viewport.
    pub outer_rect: TypedRect<u32, DevicePixel>,
    /// The chrome rect is the outer coordinates of the chrome window. It is
    /// equal to or larger than the rendering area. Usually only used for the
    /// window.moveTo/resizeTo/outerWidth/outerHeight… DOM methods.
    pub chrome_rect: (Size2D<u32>, Point2D<i32>),
    /// The device pixel ratio for view.
    pub hidpi_factor: ScaleFactor<f32, DeviceIndependentPixel, DevicePixel>,
}

[kvark]

@paulrouget ok, let's proceed with your latest scheme.

[glennw]

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

webrender/src/device.rs, line 857 at r1 (raw file):

    frame_id: FrameId,

    origin: DeviceUintPoint,

The Device code is really meant to abstract an API device, and not maintain state, where possible. We should remove this from here (explained further below).

---

webrender/src/device.rs, line 1039 at r1 (raw file):

        if let Some(dimensions) = dimensions {
            self.gl.viewport(self.origin.x as i32, self.origin.y as i32, dimensions.width as gl::GLint, dimensions.height as gl::GLint);

The bind_draw_target call is used for both the main framebuffer, and also intermediate render targets. The code as it exists will break things when intermediate targets are involved. Instead, let's change bind_draw_target() to take a rect, and pass that through from the renderer where appropriate.

---

Comments from Reviewable

[glennw]

@paulrouget Added some comments above. CI is also not happy - probably samples and/or wrench need to be updated. We'll also want to bump the version in Cargo.toml (of both WR and WR_traits) since this introduces an API change.

[bors-servo]

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

[glennw]

@paulrouget Should we keep this open for now?

[paulrouget]

@glennw yes please. I'll get back to this once I'm done with some other work.

[glennw]

Closing for now since there are so many conflicts. Feel free to re-open when rebased.