racket / plot

Other
39 stars 38 forks source link

Expose converting between plot/view/context coordinates #83

Closed dented42 closed 3 years ago

dented42 commented 3 years ago

I'm trying to integrate plot into a larger program and need a way to take coordinates in the drawing context (I'm using plot/dc) and convert them to coordinates in the plot (and back again).

I'm aware of the word done with set-mouse-event-callback etc. but I don't think using snips makes sense for my program (I'm using lux, which just gives me a drawing context).

dented42 commented 3 years ago

Given how plot is architected I'm unsure of the best way to do this. I have made a list of (possibly terrible) ideas.

A metrics objects might be a struct or object along the lines of:

(struct plot-metrics (x-min         ; real
                      x-max         ; real
                      y-min         ; real
                      y-max.        ; real
                      plot->view/x  ; invertible-function
                      view->dc/x    ; invertible-function
                      plot->view/y  ; invertible-function
                      view->dc/y )) ; invertible-function

Using axis transforms instead of invertible-functions directly might be more appropriate?

You could also expose the internal 2D-Plot-Area% instead of a metrics object, but that might force you to expose more of your implementation than you would like.

alex-hhh commented 3 years ago

The main challenge with this request (and #7 as well) is designing the new API -- unfortunately, the design of the plot package does not make this easy, since there are many entry points to creating plots, and all of them need to support all the features (if it makes sense for them, of course).

Returning a metrics object would work for plot/dc since it currently returns void, so the API change will likely be backwards compatible, but a similar request was made here for plots which produce picts: https://github.com/racket/plot/issues/7#issuecomment-650174050. In particular, this feature would need to work with:

In addition to that, there are some special plot functions in plot/pict and plot/bitmap intended for use during Scribble and web page rendering, and perhaps these would need extending too...

I think the main plot , along with plot-frame and plot-file would not need to support this functionality, but I may be wrong.

And, of course, it would also be nice if the API change would be made in such a way that the 3D variants of these functions could also be updated to provide similar functionality.

rfindler commented 3 years ago

Would it make sense if there were a new function that accepted only a plot and returned information about how drawing coordinates mapped into plotted coordinates? Then, if there were functions that took a plot and returned a pict, or a bitmap, or a snip, you could refactor plot-pict to be the composition of a function that produces a plot with the function that turns a plot into a pict?

alex-hhh commented 3 years ago

Well, all the non-GUI plot functions, such as plot-pict and plot-bitmap are implemented in terms of plot/dc, so we can update plot/dc to return the plot-metrics as it was suggested, since that function currently returns void. Internally, plot-pict would have immediate access to this structure, since it just calls plot/dc, but plot-pict returns a pict and we cannot change it to return two values, as that would be a backwards-incompatible change.

We could just leave plot-pict as-is, and write a new plot-pict-and-return-metrics (needs a better name), which would return two values, the pict and the metrics, but this would just multiply the number of entry functions in the plot package, since we would need a plot-bitmap-and-return-metrics as well. This would not be my preference, since these functions have lots of arguments, and there would be a lot of code duplication...


On a slightly separate note, @dented42 , could you describe what are you trying to achieve overall (e.g. the application that you are trying to build), perhaps there is a solution that could be implemented with existing plot functionality...

rfindler commented 3 years ago

Oh, I see. Probably best to not change the plot/dc api. I know that people probably don't rely on it returning void, but it does seem like it would not be hard to make a new function that takes the same arguments as plot/dc but returned the metrics. I see how it might be viewed as inconvenient to have to call two different functions all the time, one to draw the plot and one to get the metrics information and I can see how that might lead to one wanting to combine things.

What about a combination that was a new keyword argument that, if supplied, caused each of these functions to return two values (whatever it returned before and also the metrics)?

As for code duplication, I guess the headers get long and are duplicated but perhaps this could be done with a macro? (Or maybe it is already?)

alex-hhh commented 3 years ago

If we add a keyword argument, my preference would be to make it an optional "output parameter" using a box, like so:

(let ([metrics-boxed (box #f)])
  (define pict (plot-pict ... #:metrics metrics-boxed))
  (define metrics (unbox metrics))
  ;; use metrics here
  )

A similar technique is already used by the snip% get-extent method...

rfindler commented 3 years ago

get-extent does that because it was ported from C code. The extra box feels clunky in Racket (at least to me) -- it requires an extra step compared to just accepting multiple values as a result.

Robby

On Wed, Feb 17, 2021 at 11:42 PM Alex Harsányi notifications@github.com wrote:

If we add a keyword argument, my preference would be to make it an optional "output parameter" using a box, like so:

(let ([metrics-boxed (box #f)]) (define pict (plot-pict ... #:metrics metrics-boxed)) (define metrics (unbox metrics)) ;; use metrics here )

A similar technique is already used by the snip% get-extent method...

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/racket/plot/issues/83#issuecomment-781071136, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBNMAWNP3Y3XU6MB2IRGTS7SSEDANCNFSM4XZFVICA .

samth commented 3 years ago

The extra box is not great, but it seems better than an API that changes how many values it returns based on a keyword argument. That would be an awkward API to encode in Typed Racket, which is both relevant for plot and (in my opinion) a sign of an API that's awkward to use as well.

mfelleisen commented 3 years ago

I came to the same conclusion as Sam yesterday as I saw the first post about a "variable result" signature. But I took one extra step: the "forced" use of a box in this situation suggests a weakness in the type system.

When I faced a similar problem with big-bang event handlers for universe communications, I decided to go with distinct result structs. This still isn't idea but slightly better for types than multiple values and tons better than a box.

rfindler commented 3 years ago

I think the goal was to avoid adding another function, but maybe that's the right thing given the type system issues. That is, a new function that returns the same procedure that plot/dc currently returns but also returns the metrics.

mfelleisen commented 3 years ago

I am not proposing a new function. I am proposing

f : A x [#:metric B] -> void u metric

where we "know" that metric shows up only when an optional argument of type B is supplied.

rfindler commented 3 years ago

Oh! I see. That's better. I misunderstood.

alex-hhh commented 3 years ago

Hi @mfelleisen ,

Unfortunately I am not familiar with this notation: f : A x [#:metric B] -> void u metric , I suspect this is a function that takes two arguments, A and B, with B being an optional keyword argument, but I am unsure about the return value. Does void u metric mean:

  1. always return two values, void and metric
  2. returns either void or metric?
  3. when B is false, it returns one value, void , and when B is true it returns two values, void and metric?

Thanks, Alex.

mfelleisen commented 3 years ago

Sorry, I should have used Typed Racket notation:

(: f (-> Integer [#:metrics String] (U Boolean Void)))
(define (f {x : Integer} #:metrics [m : String "default"]) : (U Void Boolean)
  (if (equal? m "default")
      (void)
      #false))

The (U Void Boolean) means f returns either Void or Boolean. In your case, you’d return Void or the metrics coordinate info.

dented42 commented 3 years ago

I've just been trying to build some toy interactive examples of various kinds of discrete Fourier transforms, but metrics seems like a useful thing outside of that. I've been using lux specifically, and I'd like to have a series of sample points that the user can drag around with the mouse, plotted along with a curve that approximates a signal with those samples.

alex-hhh commented 3 years ago

Hi @dented42 ,

You can already build these interactive Fourier transforms using plot snips and the GUI library. It is just that you cannot implement them using the lux + plot/dc combination.

If you want to keep using lux, you could still use a plot snip, where you "manually" send the mouse events and invoke its draw method onto the target dc<%> received from lux. The two plot tests below show how snips can be used outside their normal intended use case, sending them fake events and asking them to draw themselves onto specific dc<%>s:

https://github.com/racket/plot/blob/master/plot-test/plot/tests/PRs/snip-empty-selection.rkt https://github.com/racket/plot/blob/master/plot-test/plot/tests/PRs/snip-overlay-renderers.rkt

dented42 commented 3 years ago

@alex-hhh Interesting I'll look at that. My understanding though, is that plot snip overlays can only be used to add additional things to the underlying plot and can't be used for changing/altering things that are already plotted?

alex-hhh commented 3 years ago

@dented42 , your understanding is correct, however you can have the underlying plot contain a single transparent point (or perhaps a point outside the plot range), and all the other things would be plotted using set-overlay-renderers.

bdeket commented 3 years ago

For the original question: Would it be an option to create a new Class type:

(define-type Plot-Metrics<%>
  (Class
   [get-plot-bounds (-> (Vector (Vector Real Real) (Vector Real Real)))]
   [plot->dc (-> (Vectorof Real) (Vectorof Real))]))

Then plot/dc could return an (Instance Plot-Metrics<%>) plot-bitmap, -snip and -frame, could return an (Instance (Class #:implements A% #:implements Plot-Metrics<%>) with A% equal to bitmap%, snip% or frame% resp.

for plot-pict something similar can be done with a struct, then exposing the accessors for this new structure

gist trying this out: plot/private/no-gui/plot2d.rkt

(require plot)
(define p (plot-pict (polar (λ (t) t))))
(plotpict-bounds p)
;'#(#(-28924812014055/8796093022208 ...) ...)
((plotpict-plot->dc p) #(0 0))
;'#(164.24478333605447 99.63108095845399)

(define b (plot-bitmap (polar (λ (t) 1))))
(send b plot->dc #(0 0))
;'#(220.99828569680338 177.5)
alex-hhh commented 3 years ago

Hi @bdeket , I like your proposal, and I think it would work. I don't think we would need to change plot-frame and plot-file, while plot/dc, plot-pict, plot-bitmap could be extended as you suggest and we can also update the 2d-plot-snip% class returned by plot-snip to implement the plot-metrics interface.

@rfindler you had some concerns about changing the return type of plot/dc away from void, what do you think about this proposal?

Also, does anyone else see a problem with plot-pict returning a struct extending pict, but not a pict in itself? (and similar for plot-bitmap)

alex-hhh commented 3 years ago

Hi @dented42 , @bdeket has implemented what you requested as part of the #90 pull request. Could you try these changes and see if they are sufficient for what you wanted to do with the plot package and lux? This is the branch that contains the changes: https://github.com/bdeket/plot/tree/plotmetrics

rfindler commented 3 years ago

I agree that a function that used to return void now returning something useful isn't the end of the world. It is backwards incompatible but it seems unlikely to be relied on. But if it were to be relied on, it'd be something like someone who wrote a function with a contract where that function's contract says it returns void, but it really returns whatever the plot function returns (because it just happens to call that function last). This would end up with that person's function getting blame where it used to work fine. The types might be more of a problem, tho. (Not sure if this library has a typed version.)

(I got email with a comment that I'm replying to now but I don't see that comment. Not sure if that's because I'm doing this wrong or what.)

dented42 commented 3 years ago

Late to the party again, alas. But I should chip in to say that the proposed/discussed/accepted solution covers my needs quite elegantly.