KittyCAD / modeling-app

The KittyCAD modeling app.
https://kittycad.io/modeling-app/download
MIT License
357 stars 29 forks source link

Remove "Building scene..." #3499

Closed franknoirot closed 2 weeks ago

franknoirot commented 4 weeks ago

Blocking closing #3385. I've fixed the first part of that test by fixing navigation between files with #3498. However, in order to test there is a re-execution we want to see a loading spinner between file navigations, and that is no longer occurring for some reason.

We definitely want it back to hide the model being built up since it can be pretty jump and jittery.

jessfraz commented 4 weeks ago

I actually hate that building scene thing so this is a feature imo On Aug 16, 2024, at 17:28, Frank Noirot @.***> wrote: Blocking closing #3385. I've fixed the first part of that test by fixing navigation between files with #3498. However, in order to test there is a re-execution we want to see a loading spinner between file navigations, and that is no longer occurring for some reason. We definitely want it back to hide the model being built up since it can be pretty jump and jittery.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

jessfraz commented 4 weeks ago

Like try loading a huge file and you’ll be begging to see the progress versus nothing

jessfraz commented 4 weeks ago

Try a 9x18 Lego

jessfraz commented 4 weeks ago

The user is going to see shit reexecute when they change code anyways so I think this screen doesn’t do shit

franknoirot commented 3 weeks ago

Okay that's fine, it simplifies some mind-bending stuff in our code base if we can get it out of there (cc @lf94 I think we're going to scrap the "Building scene..." loading step, read above). I think I just need to sort out how to confirm re-execution has occurred and then #3385 will be good to close. It's definitely re-executing but our test doesn't capture that.

jessfraz commented 3 weeks ago

or at least like make it semi-transparent so you can see shit thru it

lf94 commented 3 weeks ago

The building scene is only there to sell the system is fast.

Without this, modeling-app is clearly building up the object. killing any semblance of speed.

Business Lee says we should really keep it.

Technical Lee says yeah I would WAY prefer we just fix the speed issue but I'm not sure we can right now!

Casual Lee says yeah let's rip 'er out I'm not a fan of the hack either.

The internal struggle is real yo

jessfraz commented 3 weeks ago

Have now talked to people w complex kcl and all want this gone every where so yeah let’s def remove , people would rather see it executing than a loader

jessfraz commented 3 weeks ago

Users hate it

jessfraz commented 3 weeks ago

If kcl is slow let’s make it fast, no loaders

jessfraz commented 3 weeks ago

If the tradeoff is users see the model executing before we make it faster I’m fine w that

jessfraz commented 3 weeks ago

Because in some cases you are instead watching a loader with zero progress for 45 seconds and have no idea if it failed or not which is just hiding the pain and making it worse

jessfraz commented 3 weeks ago

And then people blame and think it’s the stream, anyways tldr, kill it

jessfraz commented 3 weeks ago

also @adamchalmers and ive now recruited @alteous to help on perf things so we gots people trying to make it better dont worry, just eff the loaders, its too opaque what is happening on large files

lf94 commented 3 weeks ago

Let's do itttt :rocket:

franknoirot commented 3 weeks ago

Hey I think this is a great idea, but could it wait until after the electron port is done? I would consider ripping that out a sidequest-on-sidequest situation.

jessfraz commented 3 weeks ago

up to you all as long as its right after :)

Irev-Dev commented 2 weeks ago

Oh didn't realise this was a post electron thing. I'll make a status for that.

lf94 commented 2 weeks ago

this is done i just gotta push it

lf94 commented 2 weeks ago

https://github.com/KittyCAD/modeling-app/pull/3691