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

Where to store pipeline specific options for scrolling behavior #425

Closed
paulrouget opened this issue Sep 29, 2016 · 8 comments
Closed

Where to store pipeline specific options for scrolling behavior #425

paulrouget opened this issue Sep 29, 2016 · 8 comments

Comments

@paulrouget
Copy link
Collaborator

@paulrouget paulrouget commented Sep 29, 2016

We want to implement something like this: iframe.setOverscrollOptions(options);.

I managed to send this information to WebRender.

In render_backend.rs, I get this message: ApiMsg::SetOverscrollOptions(pipeline_id, options).

Later, in frame.rs, in scroll(), I want to access these options.

I don't know in which struct I should store the options in render_backend.rs to make it accessible in frame.rs.

I only want these options to affect how the document of the pipeline scroll within the iframe. Not pipeline's inner scrollable layers.

@paulrouget
Copy link
Collaborator Author

@paulrouget paulrouget commented Sep 29, 2016

@glennw
Copy link
Member

@glennw glennw commented Sep 29, 2016

Instead of a new message, could you pass it through as an extra parameter to SetRootStackingContext message? (This is called once per pipeline here - https://github.com/servo/webrender/blob/master/webrender/src/render_backend.rs#L152)

Then, you can easily store it in the scene pipeline struct here:
https://github.com/servo/webrender/blob/master/webrender/src/scene.rs#L201

Would that work for your requirements?

@paulrouget
Copy link
Collaborator Author

@paulrouget paulrouget commented Sep 30, 2016

could you pass it through as an extra parameter to SetRootStackingContext message

I could, but I want to be able to change the overscroll options anytime. When is SetRootStackingContext called?

@paulrouget
Copy link
Collaborator Author

@paulrouget paulrouget commented Sep 30, 2016

I stored the overscroll options in ScenePipeline. Now, in frame.scroll(), I need to access these options. It doesn't sound possible within Frame to reach the scene.

RenderBacker, where scene is accessible, calls iframe.scroll(), but at this point, we don't know which pipeline is about to get scrolled.

I'm stuck.

Should I pass scene as a paramater to frame.scroll()?

@glennw
Copy link
Member

@glennw glennw commented Oct 3, 2016

In general the idea is that there is a step which takes the relevant information from the Scene structure and builds the Frame from it. This is mostly handled in the flatten() method in frame.rs. There is a step in this recursive function where it encounters a new iframe / pipeline and extracts the information from the scene. This is probably the right place to put it.

Is your code in a branch pushed to GH? It might be useful to take a look and see how it's being used - how often will these overscroll options be changed? What is it that controls / defines the overscroll options?

@paulrouget
Copy link
Collaborator Author

@paulrouget paulrouget commented Oct 4, 2016

Is your code in a branch pushed to GH?

See:

how often will these overscroll options be changed?

No often. Usually, after the user navigated (clicked on link or went back/fwd).

What is it that controls / defines the overscroll options?

For now, the option tells where the user can overscroll (top, right, bottom, left). Later, we want to introduce some more complex options:

dictionary OverscrollOptions {
  boolean enabled;
  float minPanDistanceRatio;
  float springFriction;
  float springStiffness;
  float stopDistanceThreshold;
  float stopVelocityThreshold;
}
@glennw
Copy link
Member

@glennw glennw commented Mar 21, 2017

@paulrouget I think this can be closed now?

@paulrouget
Copy link
Collaborator Author

@paulrouget paulrouget commented Mar 22, 2017

Yes. I might get back to this later. But as of now, it's not relevant anymore.

@paulrouget paulrouget closed this Mar 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.