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

Fix hang on using IGNORE with --sample-masks option #98

Merged
merged 8 commits into from
Feb 26, 2020

Conversation

Mr4k
Copy link
Contributor

@Mr4k Mr4k commented Jan 27, 2020

Checklist

  • I have read the Contributor Guide
  • I have read and agree to the Code of Conduct
  • I have added a description of my changes and why I'd like them included in the section below

Description of Changes

Resolves the hang issue in the discussion of #86. I will add also add sample mask tests tomorrow.
The hang can be reproduced by running cargo run --release -- --sample-masks ALL IGNORE -o imgs/out.png generate imgs/1.jpg imgs/1.jpg on current master

//random candidates
for _ in 0..m_rand {
let rand_map = (rng.gen_range(0, example_maps.len())) as u32;
let mut rand_map = (rng.gen_range(0, example_maps.len())) as u32;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem was if we chose a map which was ignored, we'd keep looping forever here

because no part of the image would be valid

Copy link
Contributor Author

@Mr4k Mr4k Jan 27, 2020

Choose a reason for hiding this comment

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

I'm wondering if it may be better to change this whole method to sample from a list of valid locations across images rather than the current draw / check loops. I think in the case of large images with tiny valid sample masks the current method could take a while to find a valid pixel. But I'm not sure how common that type of case is and this is the smaller code change.

It might also be possible to maybe just filter out the ignore images at the beginning of the main_resolve_loop or even in lib.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think filtering out the ignore images once at the beginning of the resolve loop makes the most sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

//random candidates
for _ in 0..m_rand {
let rand_map = (rng.gen_range(0, example_maps.len())) as u32;
let mut rand_map = (rng.gen_range(0, example_maps.len())) as u32;
Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think filtering out the ignore images once at the beginning of the resolve loop makes the most sense.

@mergify mergify bot merged commit 4a3aab2 into EmbarkStudios:master Feb 26, 2020
This was referenced Feb 26, 2020
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