Skip to content
This repository has been archived by the owner on Feb 16, 2023. It is now read-only.

adding a surface observer interface #8

Open
wants to merge 1 commit into
base: mir-1.9-miroil
Choose a base branch
from

Conversation

erlend-g
Copy link

  1. Adding interface surface observer interface
  2. Adding a wrapper for Surface

Copy link
Member

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this code will be used on a single thread, and if that's right it isn't threadsafe.

void window_resized_to(mir::scene::Surface const* surf, mir::geometry::Size const& window_size) override;

private:
std::shared_ptr<miroil::SurfaceObserver> listener;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be const?

int query(MirWindowAttrib attrib) const;

private:
std::shared_ptr<mir::scene::Surface> wrapped;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be const?


private:
std::shared_ptr<mir::scene::Surface> wrapped;
std::unordered_map<std::shared_ptr<miroil::SurfaceObserver>, std::shared_ptr<miroil::SurfaceObserverImpl>> observers;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't access to this be protected by (at least) a mutex?

It is probably best to copy the SurfaceObservers implementation (by using BasicObservers<> to achieve threadsafety.)

@AlanGriffiths
Copy link
Member

I don't think this code will be used on a single thread, and if that's right it isn't threadsafe.

For a discussion about this design context see https://accu.org/journals/overload/22/124/griffiths_2040/

Some of the names have changed, and some of the code moved, but the design is the same.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants