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

Enforce magnifications via task type #4101

Merged
merged 17 commits into from
May 28, 2019
Merged

Enforce magnifications via task type #4101

merged 17 commits into from
May 28, 2019

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented May 21, 2019

@youri-k Can you update the migration guide? A datastore update is probably not necessary, is it?

Additionally, to the mag enforcement, I also added an "active resolution" hint to the info tab. I think, this is quite useful (at least, it was for testing :)).

URL of deployed dev instance (used for testing):

Steps to test:

  • edit a task type and check "enforce magnifications"
  • fill min and max (you can also try to omit one value, as this should work, too)
  • create a task for that task type
  • trace the task and ensure that you can only zoom within that range

Issues:


@philippotto philippotto self-assigned this May 21, 2019
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

Backend LGTM (please update the PR number in evolution file + add migration guide entry)

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Code looks really good, the active resolution is very helpful! 👍

I started testing, but encountered an issue right away, so I first wanted to let you check that before continuing the testing:

@philippotto
Copy link
Member Author

I started testing, but encountered an issue right away, so I first wanted to let you check that before continuing the testing:

  • I created a task type with a min resolution of 4 and a max of 512
  • I then created a task with that task type and opened said task
  • The task loads at resolution 2-2-2 and I cannot zoom (neither in nor out)

Oh, very good catch! I fixed it now.

Also, do you notice any performance regressions when zooming? Locally, it appeared a bit laggier to me, but the reason might as well be that I used wk predominantly on deployed instances (where the code is optimized by compilation). In benchmarks, the code wasn't conspicuous.

@philippotto
Copy link
Member Author

Backend LGTM (please update the PR number in evolution file + add migration guide entry)

@youri-k Could you do this? :)

@daniel-wer
Copy link
Member

Works very well!
I didn't notice any performance regression when zooming, I'm also getting similar FPS during zooming on master and this branch :)

@philippotto philippotto merged commit 0b6b8b0 into master May 28, 2019
@normanrz normanrz deleted the enforce-mags branch August 12, 2019 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to enforce specific mag(s) for tasks
4 participants