cloudhead / rx

👾 Modern and minimalist pixel editor
https://discord.gg/xHggPjfsS9
GNU General Public License v3.0
3.09k stars 109 forks source link

Cannot open files (view #1 must exist and have an associated snapshot) #56

Closed pop closed 4 years ago

pop commented 4 years ago

Release: Master (commit 74fac87f567b4a3762738b3a0373b0d92051bb37) Platform: Fedora Linux + Wayland Bug: Cannot open existing files

I can open new files just fine but when I try to open an existing .png file rx pops open a window and then crashes.

$ RUST_BACKTRACE=full cargo run -- -v foo.png 
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/rx -v foo.png`
2020-01-11 09:21:46,841 DEBUG [rx] options: Options { width: 1280, height: 720, resizable: true, headless: false, source: None, exec: Normal, debug: false }
2020-01-11 09:21:46,871 INFO  [rx] framebuffer size: 1281x720
2020-01-11 09:21:46,872 INFO  [rx] scale factor: 1.001165747642517
2020-01-11 09:21:46,874 DEBUG [rx::session] state: Initializing -> Running
[... snip ...]
2020-01-11 09:21:46,878 DEBUG [rx::session] source: /home/pop/Projects/src/github.com/cloudhead/rx/.rxrc
2020-01-11 09:21:46,878 DEBUG [rx::session] rx v0.3.1
2020-01-11 09:21:46,898 DEBUG [rx::session] load: "foo.png"
2020-01-11 09:21:46,917 INFO  [rx::session] "foo.png" 16384 pixels read
thread 'main' panicked at 'view #1 must exist and have an associated snapshot', src/libcore/option.rs:1188:5
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
[... snip ...]
  12: core::panicking::panic_fmt
             at src/libcore/panicking.rs:84
  13: core::option::expect_failed
             at src/libcore/option.rs:1188
  14: core::option::Option<T>::expect
             at /rustc/3a3f4a7cbaff09722b8c7cc8f09ce86ff5f953a3/src/libcore/option.rs:348
  15: rx::resources::Resources::get_snapshot
             at src/resources.rs:105
  16: rx::gfx::Renderer::handle_effects
             at src/gl/mod.rs:809
  17: <rx::gfx::Renderer as rx::renderer::Renderer>::init
             at src/gl/mod.rs:382
  18: rx::init
             at ./src/lib.rs:193
  19: rx::execute
             at src/main.rs:126
  20: rx::main
             at src/main.rs:36
  21: std::rt::lang_start::{{closure}}
             at /rustc/3a3f4a7cbaff09722b8c7cc8f09ce86ff5f953a3/src/libstd/rt.rs:67
[... snip ...]

I'm digging into this but I'm not making much progress. I'll gladly try to fix it if you give me some hints on where the solution might go :wink:

cloudhead commented 4 years ago

Hey I think this was a regression and should be fixed in master. Nevermind, you're right. Looking into it now.

cloudhead commented 4 years ago

Ok, the issue was introduced with the change in ordering between init and blank. The problem is simple:

  1. a blank view gets created, this queues a message for the renderer to create a view.
  2. a file is loaded (all in the same frame, because it's passed in through the CLI), this causes the blank view to be destroyed.
  3. the renderer runs, tries to add the view based on the message, but the view doesn't exist, since it was destroyed
  4. :boom:
cloudhead commented 4 years ago

Fixed: e1a0858f708f5505406d0a41025b0233eeb8488e

I think I'll do a v0.3.2 release with these fixes!

pop commented 4 years ago

Issue is fixed in master. Thanks @cloudhead :1st_place_medal:

cloudhead commented 4 years ago

I'm kind of annoyed that my tests didn't catch this. I need to put more time into that..

pop commented 4 years ago

If you want help writing more tests I'm interested in helping out. Let me know what kind of coverage you feel is missing.

cloudhead commented 4 years ago

@pop that's great to hear, I'll create an issue with some details on what's missing!