fix work with TRACK_POINTERS enabled #358

2021-04-30T11:21:54Z

Dushistov commented

2021-04-30 11:21:54 +00:00

(Migrated from github.com)

fixes #357

biodranik (Migrated from github.com) approved these changes 2021-04-30 12:02:05 +00:00

drape/overlay_tree.hpp

					
				@ -74,2 +74,3 @@

				  void Remove(ref_ptr<OverlayHandle> handle);

				  //! \return true if tree completely invalidated and next call has no sense

				  bool Remove(ref_ptr<OverlayHandle> handle);

				  void EndOverlayPlacing();

biodranik (Migrated from github.com) commented

2021-04-30 11:57:10 +00:00

Комментарий, что возвращается, не помешал бы.

drape/render_bucket.hpp

					
				@ -35,2 +35,3 @@

				  void RemoveOverlayHandles(ref_ptr<OverlayTree> tree);

				  //! \return true if tree completely invalidated and next call has no sense

				  bool RemoveOverlayHandles(ref_ptr<OverlayTree> tree);

				  void SetOverlayVisibility(bool isVisible);

biodranik (Migrated from github.com) commented

2021-04-30 11:57:40 +00:00

И здесь.

drape_frontend/render_state_extension.cpp

biodranik (Migrated from github.com) commented

2021-04-30 12:00:22 +00:00

static_assert?

Dushistov (Migrated from github.com) reviewed 2021-04-30 12:32:48 +00:00

drape/render_bucket.hpp

					
				@ -35,2 +35,3 @@

				  void RemoveOverlayHandles(ref_ptr<OverlayTree> tree);

				  //! \return true if tree completely invalidated and next call has no sense

				  bool RemoveOverlayHandles(ref_ptr<OverlayTree> tree);

				  void SetOverlayVisibility(bool isVisible);

Dushistov (Migrated from github.com) commented

2021-04-30 12:32:48 +00:00

Done

Dushistov (Migrated from github.com) reviewed 2021-04-30 12:32:57 +00:00

drape/overlay_tree.hpp

					
				@ -74,2 +74,3 @@

				  void Remove(ref_ptr<OverlayHandle> handle);

				  //! \return true if tree completely invalidated and next call has no sense

				  bool Remove(ref_ptr<OverlayHandle> handle);

				  void EndOverlayPlacing();

Dushistov (Migrated from github.com) commented

2021-04-30 12:32:57 +00:00

Done

Dushistov (Migrated from github.com) reviewed 2021-04-30 12:34:38 +00:00

drape_frontend/render_state_extension.cpp

Dushistov (Migrated from github.com) commented

2021-04-30 12:34:38 +00:00

depthLayer не constexpr, поэтому assert(depthLayer < kStateExtensions.size()) не может быть вычислен в compile-time

depthLayer не constexpr, поэтому `assert(depthLayer < kStateExtensions.size())` не может быть вычислен в compile-time

biodranik (Migrated from github.com) reviewed 2021-04-30 12:55:19 +00:00

drape_frontend/render_state_extension.cpp

biodranik (Migrated from github.com) commented

2021-04-30 12:55:19 +00:00

Верно, я сообразил и вроде удалял коммент, но гитхаб глюкнул.

Dushistov commented

2021-04-30 22:31:56 +00:00

(Migrated from github.com)

Все еще иногда сообщает об удалении unique_ptr в то время как есть соответсвующие ref_ptr разделяющие с ним указатель. Это происходит при остановке приложения, когда вызывается BackendRenderer::OnContextDestroy.
Так в очереди на обработку еще могут лежать MapShape у которых есть ссылка на данные TextureManager,
который как раз очищается в BackendRenderer::OnContextDestroy. Но это вроде бы не критично, так как BackendRenderer::OnContextDestroy вызывается только при завершении приложения и перед удалением чего-нибудь
там включается фильтр для всего связанного с графическим контекстом.

Все еще иногда сообщает об удалении `unique_ptr` в то время как есть соответсвующие `ref_ptr` разделяющие с ним указатель. Это происходит при остановке приложения, когда вызывается `BackendRenderer::OnContextDestroy`. Так в очереди на обработку еще могут лежать `MapShape` у которых есть ссылка на данные `TextureManager`, который как раз очищается в `BackendRenderer::OnContextDestroy`. Но это вроде бы не критично, так как `BackendRenderer::OnContextDestroy` вызывается только при завершении приложения и перед удалением чего-нибудь там включается фильтр для всего связанного с графическим контекстом.

👍 1

vng (Migrated from github.com) reviewed 2021-05-01 07:22:18 +00:00

drape/overlay_tree.cpp

					
				@ -160,1 +161,4 @@

				      Clear();

				    return true;

				  }

vng (Migrated from github.com) commented

2021-05-01 07:22:13 +00:00

Hm, Is it a valid state or something went wrong and we made this patch? When

m_frameCounter == kInvalidFrame && !IsEmpty()

Hm, Is it a valid state or something went wrong and we made this patch? When ``` m_frameCounter == kInvalidFrame && !IsEmpty() ```

Dushistov (Migrated from github.com) reviewed 2021-05-01 11:42:05 +00:00

drape/overlay_tree.cpp

					
				@ -160,1 +161,4 @@

				      Clear();

				    return true;

				  }

Dushistov (Migrated from github.com) commented

2021-05-01 11:42:05 +00:00

Yes, there are several places where m_frameCounter setted to kInvalidFrame without call of Clear(), for example OverlayTree::SetVisualScale.
And this is ok, because of only OverlayTree::Remove call can cause dangling ref_ptr.
So with something like map scale change and move we can get kInvalidFrame via OverlayTree::SetVisualScale and only then call to OverlayTree::Remove before start of next frame. And in that case we get danging ref_ptr without extra Clear call.

Yes, there are several places where `m_frameCounter` setted to `kInvalidFrame` without call of `Clear()`, for example `OverlayTree::SetVisualScale`. And this is ok, because of only `OverlayTree::Remove` call can cause dangling `ref_ptr`. So with something like map scale change and move we can get kInvalidFrame via `OverlayTree::SetVisualScale` and only then call to `OverlayTree::Remove` before start of next frame. And in that case we get danging `ref_ptr` without extra `Clear` call.

This repo is archived. You cannot comment on pull requests.

No reviewers

biodranik