SketchUp / api-issue-tracker

Public issue tracker for the SketchUp and LayOut's APIs
https://developer.sketchup.com/
38 stars 10 forks source link

Page#selelected_page= : Method to select a page WITHOUT animation #876

Open Fredosixx opened 1 year ago

Fredosixx commented 1 year ago

Currently, Pages#selected_page=page sets the specified page as if te user would click on its tab, that is with an animation hich may last several seconds.

Could we have a method that bypasses the animation?

This would be easier to manage scenes from the API.

DanRathbun commented 1 year ago

Could we have a method that bypasses the animation?

We've been doing this for years already using the current API features.

What we do is save the current state, switch off the scene transitions, and then restore them when done.

opts = Sketchup.active_model.options["PageOptions"]
prev = opts["ShowTransition"]
opts["ShowTransition"]= false
#
# Do page tasks here ... loop through pages, etc.
#
opts["ShowTransition"]= prev if opts["ShowTransition"] != prev
Fredosixx commented 1 year ago

Good workaround, worth knowing.

It does however impact the order of observers frame_change_observer and pages_observer, which are already very messy (events sent several times and order difficult to understand), which is for instance what you want to both track the changes of existing scenes and the selection of scenes (which apparently triggers a change in content).

DanRathbun commented 1 year ago

_It does however impact the order of observers frame_change_observer ..._

Yes, @Eneroth3 described the bug in Issue 244 : Fire FrameChangeObserver at least twice for all scene changes

In the scenario described however, since it is your code switching off the scene transitions, then the current page's properties can be cached before change the page. You'd need to add the functionality to pass that data into a frame change observer.

... _and pages_observer_, ...

I'm assuming that #onActiveSectionPlaneChanged would be most likely affected.

What other PagesObsever callbacks would fire when changing a scene ?


Regarding a new non-animate scene page changer, it is big no-no in Ruby to have more than 1 argument for an assignment method that uses "=".

So it would have to be named like: #set_selected_page( page, animate: false ) (or similar.)

Fredosixx commented 1 year ago

For info,

when you change a page, the FrameChange observer fires with the percent of animation.

But in addition, if you have the Pages observer on, you get 2 onContentsModified events, whereas you are just changing the current scene. These events fire during the animation. I can identify them because the Frame Change with percent_done 0 has been received before. Now with the suppression of animation, I get the onContentsModified notifications before I get the Frame Change with percentdone = 1. So, difficult to indentify a true change in a page versus a simple change of page. This is the side effect.

And indeed, the Pages observer is totally silent when you change the flags of usage (camera, style, layers, etc...). NO event at all. I think this has already been logged in the issue database.

DanRathbun commented 1 year ago

But in addition, if you have the Pages observer on, you get 2 onContentsModified events, whereas you are just changing the current scene. ... So, difficult to identify a true change in a page versus a simple change of page. This is the side effect.

Yep ... Thomas has detailed these internal problems in this request for a new PagesObserver callback 4 years ago ...

And indeed, the Pages observer is totally silent when you change the flags of usage (camera, style, layers, etc...). NO event at all. I think this has already been logged in the issue database.

Yea. I logged this request 5 years ago ...

Fredosixx commented 1 year ago

Thanks. I'll look back at the status in 6 years then....

DanRathbun commented 1 year ago

FYI: I listed the quirkiness of PagesObserver#onContentsModified in this post ... https://github.com/SketchUp/api-issue-tracker/issues/26#issuecomment-691744240

thomthom commented 1 year ago

Could we have a method that bypasses the animation?

Can you elaborate on why you need the API to change the active scene without using the scene's transition properties?

Fredosixx commented 1 year ago

to speed time.

thomthom commented 1 year ago

Why is that needed for your extension? What is the background for this? What is the extension doing at a higher level?

We need to understand the context of feature requests that we beyond just parity with the UI.

Fredosixx commented 1 year ago

The plugin uses a list of scenes to manage extra properties and the user can scroll it.

Dan's trick is OK for removing the animation, but then you have to fix the the mess with the observers.

DanRathbun commented 1 year ago

@thomthom Can you elaborate on why you need the API to change the active scene without using the scene's transition properties?

The need centers around the fact that the Style class is not fully exposed in it's own right. Instead we must access a scene page's style settings via the RenderingOptions object.

But we cannot access or change the options independently from the current model options. Contrary to what the API docs for Page#rendering_options implies, getting this object and making changes does not work. (I think that there is an open issue on this.)

Instead, in order to make scene page style changes, we must switch to a page via Pages#selected_page= then access the model's rendering options and make changes to this, then be sure that the page is set to remember styles or rendering options (one of them may not work as advertised) .. anyway then we must do an update on the page with the correct update flags.

I think that this also is similar to changing the camera for a page. The page to be changed must be the current selected page, the camera changed, then the page updated.

If an extension is doing some of these updates to many or all pages, then a loop must be wrapped within a block like I've shown above. Otherwise some properties such as some style and rendering options are interpolated during the animation.

If an extension was going to write out images of all scene page views to disk, the result might be somewhere in the midst of the transition with a mix of styling between the start page and target page.

Anyway, the example of temporarily switching off scene page transitions is not new. We've shown it in the forums for years as SketchUp coding newbs encounter the challenges described in this issue thread.

DanRathbun commented 1 year ago

Not sure where or if I remember correctly, but perhaps we've discussed a "quick iterator" for the Pages collection class like:

pages = Sketchup.active_model.pages
pages.cycle do |page|
  # page would now be the selected page after instant transition
end

An extension or author library could use a wrapper ...

    # Cycle through model pages with scene transitions off.
    # Code is responsible to wrap in undo operation if needed.
    #
    # @param model [Sketchup::Model] Defaults to the active model.
    # @yield page [Sketchup::Page] Each page in turn.
    # @return [nil]
    def cycle_pages(model = Sketchup.active_model)
      unless block_given?
        fail(ArgumentError,'Block required',caller)
      end
      opts = model.options["PageOptions"]
      # Save the current page:
      pages = model.pages
      old_page = pages.selected_page
      # Save the current global scene transition setting:
      prev = opts["ShowTransition"]
      # Switch off the global scene transition setting:
      opts["ShowTransition"]= false
        #
        pages.each do |page|
          pages.selected_page= page
          # page should now be the selected page after instant transition
          yield page # Do page tasks in block
        end
        #
        # Restore the original selected page:
        pages.selected_page= old_page
        # Restore the global scene transition setting:
      opts["ShowTransition"]= prev if opts["ShowTransition"] != prev
      # No particular return value:
      nil
    end

A refinement could be used ...

module SomeAuthor
  module PagesCycleRefinement

    refine ::Sketchup::Pages do

      # Cycle through model pages with scene transitions off.
      # Code is responsible to wrap in undo operation if needed.
      #
      # @yield page [Sketchup::Page] Each page in turn.
      # @return [self] This pages collection itself.
      def cycle
        unless block_given?
          fail(ArgumentError,'Block required',caller)
        end
        opts = self.model.options["PageOptions"]
        # Save the current page:
        old_page = self.selected_page
        # Save the current global scene transition setting:
        prev = opts["ShowTransition"]
        # Switch off the global scene transition setting:
        opts["ShowTransition"]= false
          #
          self.each do |page|
            self.selected_page= page
            # page should now be the selected page after instant transition
            yield page # Do page tasks in block
          end
          #
          # Restore the original selected page:
          self.selected_page= old_page
          # Restore the global scene transition setting:
        opts["ShowTransition"]= prev if opts["ShowTransition"] != prev
        # Return the collection object as this is not a filter:
        self
      end

    end # class refinement

  end # refinement module
end # top level namespace
thomthom commented 1 year ago

The need centers around the fact that the Style class is not fully exposed in it's own right. Instead we must access a scene page's style settings via the RenderingOptions object.

Are there any use cases outside of that? Sounds to me the real fix is to address the root cause, allowing these properties to be set without the workaround of temporarily changing active page.

I'd favor #878 over this one. We don't want to add API to enable workaround workflow, but rather focus on root cause fixes. (This is why background and context to a feature request is important.)

DanRathbun commented 1 year ago

Are there any use cases outside of that?

YES.

Other scenarios in which we need to quickly cycle through the pages collection is to:

I could likely search through the forums on ["ShowTransition"]= false and find more scenarios over the years where we've had to instruct coders to switch off the page transitions to prevent unwanted strangeness.

ADD, ... from a SketchUp forum search (I did not check SketchUcation):

The main quirk to avoid was that often the output would be somewhere in the midst of a transition, where the rendered view was not at the desired camera position and the render was some interpolated mix between the style of the source page and the target page. This could also result in say a section cut in the wrong location if use_section_planes= was set true.

Now, again when we speak of this, we always use the term "quick" because most often we want to do some batch processing as quickly as possible. So this is why I added the question (above) with examples of a quick Pages#cycle iterator. My question now would be are they the same feature, or should I move the block form cycle iterator to a feature request issue of it's own ? I ask because perhaps both ideas may have merit, and the cycle iterator would likely leverage the single action page selector.

Keep in mind that an issue that we have even now is that Pages#selected_page= is (I think) an asynchronous method that mimics the user clicking a scene tab. What we coders need is a synchronous page selector method that does not return until the view has completely rendered the target page. I am not convinced that the current setter method does this. I seem to recall having to use clunky workarounds involving a temporary FrameChangeObserver pattern.

Sounds to me the real fix is to address the root cause, allowing these properties to be set without the workaround of temporarily changing active page.

I am not against a better and direct pattern to change scene page properties ... and their axes, camera, rendering_options, shadow_info, and style.

But these enhancements are more directly covered by Issue #878.

I'd favor #878 over this one.

Except that I brought up the API issue with access to scene styles and rendering options years ago, (I can't remember if it was public or private,) but Chris responded something like "(shrug) This is the way it is because of the way that the core is written".

This leads me to believe that implementing #878 will be much more complex, costly and time consuming core changes than what this issue asks for ... a fast non-transition synchronous page selector method (and hopefully from that a fast non-transition block form iterator.)

We don't want to add API to enable workaround workflow, but rather focus on root cause fixes. (This is why background and context to a feature request is important.)

Sure, but we are programmers with 15 years experience using the SketchUp API and myself twice that programming seriously with 45 years engineering. We likely know what is broken in the API and what features it does not have that we cannot workaround easily or are so clunky that it diminishes the worth of the API.

I ask that you give us more credit and respect when we ask for API enhancements, as we have most probably already investigated the alternatives.

In my opinion, I should not have to waste hours defending good requests with these long explanations typed with my arthritic hands.

Fredo's idea for an instantaneous scene change method is valid and a good request, whose absence has been felt by many SketchUp coders in the past.

I feel that Extensibility Team member often assume wrongly that there must be some easily fixed "root cause bug" that will make API enhancement requests moot. This is not going to be the case when requests are made by programmers experienced especially with the SketchUp API. We know what is broken. We know what needs improvement. Give us a bit more respect, please !

P.S. - If you'd like more explanations & use cases then it may be time to update the issue templates for this tracker, a separate one for bugs, another for enhancements. The latter can make a better query for use case explanation, and you might ask if the request is for a bug workaround.

DanRathbun commented 1 year ago

Examples of coders in the past having issues with transitions, needing them disabled whilst change to a page and then doing tasks with that page rendered. Often attempts to workaround Sketchup::Pages#selected_page= being asynchronous using a FrameChangeObserver fail as the pattern is clunky, and this observer class has bugs and it's implementation is weird.

DanRathbun commented 1 year ago

NOTE: misspelled method name in issue title.