Skip to content

Conversation

Saumya-Mishra9129
Copy link
Member

@Saumya-Mishra9129 Saumya-Mishra9129 commented Jul 19, 2020

Tested , works as expected. draw by clicking on stop button is solved in bf4201f. I did what @quozl suggested in #6 (comment). @quozl @chimosky Please review.

@quozl
Copy link
Contributor

quozl commented Jul 20, 2020

Thanks. Reviewed by examining diff, with and without whitespace changes.

  • the test for the old toolbox is no longer required,
  • documentation pointer to wiki.laptop.org should be replaced,

Tested for excessive updates;

GTK_DEBUG=updates sugar-activity3

Pass. bf4201f is effective.

Tested features;

  • all the view changing buttons are initially ineffective; they should queue a redraw,

Checked appearance;

  • line drawing width regression; the constellation lines and stars are different size; they should be the same size as the Python 2 version shows, see screenshots;

20b85ce
Screenshot_ubb_2020-07-21_09:42:31

bf4201f
Screenshot_uff_2020-07-21_09:42:40

@Saumya-Mishra9129
Copy link
Member Author

all the view changing buttons are initially ineffective; they should queue a redraw,

I saw some ambiguous behaviour of view buttons. They are queuing redraw, but only when we click on main view button after clicking on a particular view button.

line drawing width regression; the constellation lines and stars are different size; they should be the same size as the Python 2 version shows, see screenshots;

Can you please tell on which display you are testing?

@quozl
Copy link
Contributor

quozl commented Jul 22, 2020

all the view changing buttons are initially ineffective; they should queue a redraw,

I saw some ambiguous behaviour of view buttons. They are queuing redraw, but only when we click on main view button after clicking on a particular view button.

Why does that happen?

line drawing width regression; the constellation lines and stars are different size; they should be the same size as the Python 2 version shows, see screenshots;

Can you please tell on which display you are testing?

In the above screenshots, QEMU/KVM with virtual display configured for 1024x768 resolution. I don't understand why you need to know this.

@Saumya-Mishra9129
Copy link
Member Author

Why does that happen?

I don't know. I haven't made any change which could result in this behaviour. You have also mentioned the issue in #6 (comment).

buttons night vision, invert display, flip l/r, draw constellations, the magnitude radio buttons, latitude changes, all time changes; do not instantly redraw; workaround is to collapse the toolbar,.

In the above screenshots, QEMU/KVM with virtual display configured for 1024x768 resolution. I don't understand why you need to know this.

Actually this issue is also introduced by that previous port to Gtk3 , Trying to look into it what could cause this behaviour.

@quozl
Copy link
Contributor

quozl commented Jul 27, 2020

Why does that happen?

I don't know. I haven't made any change which could result in this behaviour. You have also mentioned the issue in #6 (comment).

It is caused by not calling Gtk.Widget.queue_draw() in the clicked callback of the night vision button. In turn, caused by not properly porting from GTK 2. Gtk.DrawingArea is very different, and drawing methods very different. The draw callback in your branch doesn't properly use the supplied Cairo context argument; it is saved for later use by the button callbacks, by which time the (copy of) the context is unlikely to be valid. Drawing is most properly done within the draw callback, and if any button callbacks need to activate the draw callback they must call queue_draw.

Summary is that the Port to GTK 3 is incomplete.

@JuiP
Copy link
Member

JuiP commented Aug 8, 2020

Tested not yet reviewed changes; some additional observations/suggestions:

  • The activity doesnot implement collaboration so set self.max_participants = 1

  • Consider setting tooltips for activity toolbar icons

  • All the view buttons were ineffective at start. All the changes seem to be applied each time I click on the longitude button.

  • Select Object Type, Brightest Stars and choose one option eg. Adharaz , a green cross appears. I am assuming it shows the position of the selected star. Now use flip L/R option under view; the image is flipped but the green cross still points to the same area, effectively now it points to the wrong star. See below screenshots:

con-1

After flip points to wrong star:
flipped

@chimosky
Copy link
Member

@quozl said;

It is caused by not calling Gtk.Widget.queue_draw() in the clicked callback of the night vision button. In turn, caused by not properly porting from GTK 2. Gtk.DrawingArea is very different, and drawing methods very different. The draw callback in your branch doesn't properly use the supplied Cairo context argument; it is saved for later use by the button callbacks, by which time the (copy of) the context is unlikely to be valid. Drawing is most properly done within the draw callback, and if any button callbacks need to activate the draw callback they must call queue_draw.

Summary is that the Port to GTK 3 is incomplete.

Fixed queue_draw not being called in draw_cb.

@JuiP said;

  • The activity doesnot implement collaboration so set self.max_participants = 1
  • Consider setting tooltips for activity toolbar icons

When tooltips are set, clicking on the buttons doesn't activate them when they're clicked for some reason.

  • Select Object Type, Brightest Stars and choose one option eg. Adharaz , a green cross appears. I am assuming it shows the position of the selected star. Now use flip L/R option under view; the image is flipped but the green cross still points to the same area, effectively now it points to the wrong star.

This is an existing bug and wasn't created by this PR, doesn't mean it can't be fixed here but I don't think it's paramount that it's fixed here.

@quozl
Copy link
Contributor

quozl commented Feb 16, 2021

f5f7b7b ("queue draw in draw_cb") causes an infinite loop because every time GTK asks the app to draw, the app asks GTK to ask to draw again.

It is not correct to call queue_draw inside the draw callback. Instead, queue_draw must be called in each place that requires a redraw, such as the clicked callback of the night vision button.

Signed-off-by: Ibiam Chihurumnaya <[email protected]>
@chimosky
Copy link
Member

f5f7b7b ("queue draw in draw_cb") causes an infinite loop because every time GTK asks the app to draw, the app asks GTK to ask to draw again.

It is not correct to call queue_draw inside the draw callback. Instead, queue_draw must be called in each place that requires a redraw, such as the clicked callback of the night vision button.

Seems it's been called in all the necessary places, do you still notice the appearance difference in your tests?

@quozl
Copy link
Contributor

quozl commented Feb 18, 2021

Tetsed f4933fe. The view buttons remain ineffective. Here's how I fixed it for the Flip L/R button;

diff --git a/StarChart.py b/StarChart.py
index 225ec62..f643714 100644
--- a/StarChart.py
+++ b/StarChart.py
@@ -1164,6 +1164,7 @@ class ChartDisplay(Gtk.DrawingArea):
         elif (data == 'flip horizontally'):
             self.fliphorizontally = button3.get_active()
             self.plotchart()
+            self.queue_draw()
             return False
         elif (data == 'draw constellations'):
             self.drawconstellations = button4.get_active()

Probably there is a better place to put this.

switch to floor division when divisor is an int
remove redundant reversed call in loops
call self.queue_draw in plotchart for a redraw

Signed-off-by: Ibiam Chihurumnaya <[email protected]>
@quozl quozl mentioned this pull request Aug 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants