Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to Gtk+ 3 #6

Closed
wants to merge 15 commits into from
Closed

Port to Gtk+ 3 #6

wants to merge 15 commits into from

Conversation

yell0wfl4sh
Copy link

Activity is ported to GTK +3 , Cairo.

A couple of things that I was not able to fix.

  • Make gray ellipse with solid outline, see here
  • Buttons in toolbar are working but they take long time to come into effect. This see changes appear quickly change the window and come back to it. This error is also logged at the same time:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/sugar3/graphics/toolbutton.py", line 183, in do_clicked
    if self._hide_tooltip_on_click and self.palette:
AttributeError: 'ToggleButtonTool' object has no attribute '_hide_tooltip_on_click'

@quozl @chimosky please review!

@quozl
Copy link
Contributor

quozl commented Jun 22, 2018

Thanks. Reviewed to c7934d9.

make gray ellipse

I've no idea.

AttributeError: 'ToggleButtonTool' object has no attribute '_hide_tooltip_on_click'

Perhaps caused by GObject.GObject.__init__ call being made after the super().__init__ call; unusual to call an __init__ twice like that.
https://github.com/yashagrawal3/starchart/blob/c7934d910ea4376a5283186357f472d693efc0cd/StarChart.py#L200

Perhaps that class can be replaced by ToggleToolButton in sugar3?

@yell0wfl4sh
Copy link
Author

Changed to ToggleButtonTool but the behavior is still the same.

@chimosky
Copy link
Member

chimosky commented Jun 23, 2018

Thanks, reviewed to c7934d9

I get this error -- the activity runs fine though --

Traceback (most recent call last):
  File "/home/ibiam/yash/starchart/StarChart.py", line 1121, in area_expose_cb
    self.plotchart()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1663, in plotchart
    self.plot_whole_sky()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1776, in plot_whole_sky
    self.plot_all_DSOs()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1837, in plot_all_DSOs
    self.plot_DSO(strT, majA, minA, mag, px, py)
  File "/home/ibiam/yash/starchart/StarChart.py", line 2820, in plot_DSO
    self.gc.scale(int((dx-1)/2), int((dy-1)/2))
cairo.Error: invalid matrix (not invertible)
Traceback (most recent call last):
  File "/home/ibiam/yash/starchart/StarChart.py", line 1128, in timer1_cb
    self.plotchart()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1659, in plotchart
    self.plotfield()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1674, in plotfield
    self.cleararea()
  File "/home/ibiam/yash/starchart/StarChart.py", line 2900, in cleararea
    (1 / 65536.0) * self.colors[3].blue)
cairo.Error: invalid matrix (not invertible)

It'll be great if you remove the commented out code, there's too many of them.

AttributeError: 'ToggleButtonTool' object has no attribute '_hide_tooltip_on_click'

I don't know why you get this error, there's a self._hide_tootip_on_click in Toolbutton, try using a debugger to see if you can get more information.

@yell0wfl4sh
Copy link
Author

@quozl @chimosky apparently the line I added for ellipse was causing the problem, I have removed it.
I am unable to reproduce AttributeError: 'ToggleButtonTool' object has no attribute '_hide_tooltip_on_click' error.

@quozl
Copy link
Contributor

quozl commented Jun 27, 2018

@yashagrawal3, great, thanks.

@chimosky, when you pasted that traceback, you didn't press enter after the three tick marks, so the text traceback was taken by GitHub to be a language description. Useful, because the text is smaller. But the missing first line makes it harder to perceive. I'll edit.

@chimosky
Copy link
Member

@quozl, i did press enter after the three tick marks, the traceback started on the next line.

@yell0wfl4sh
Copy link
Author

@quozl @chimosky what all changes do suggest moving towards merging this pull request.

@chimosky
Copy link
Member

@yashagrawal3, thanks. Reviewed, nice job.

@quozl
Copy link
Contributor

quozl commented Jun 28, 2018

@quozl, i did press enter after the three tick marks, the traceback started on the next line.

No, you didn't. If you're looking at how it looks like now, remember you're looking at my edits. Look at the history.

@chimosky
Copy link
Member

@quozl, how would you like to proceed?, is there anything you'd like @yashagrawal3 to do?.

@quozl
Copy link
Contributor

quozl commented Jun 29, 2018

@chimosky, I'm waiting for consensus. With your involvement in the pull request, I wait for you to review the latest commit and tell me if you tested.

@chimosky
Copy link
Member

@quozl thanks, @yashagrawal3 reviewed up to eeacdb3 i get this error

Traceback (most recent call last):
  File "/home/ibiam/yash/starchart/StarChart.py", line 1121, in area_expose_cb
    self.plotchart()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1663, in plotchart
    self.plot_whole_sky()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1778, in plot_whole_sky
    self.plot_all_planets()
  File "/home/ibiam/yash/starchart/StarChart.py", line 2193, in plot_all_planets
    self.location.plot_cross()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1053, in plot_cross
    self.context.gc.set_source_rgb(self.colors[4].red, self.colors[4].green, self.colors[4].blue)
AttributeError: Location instance has no attribute 'colors'

Seems that's the first time it's been called and it wasn't initialised in that class but it was initialised in class ChartDisplay(); initialising it - same as in class ChartDisplay() i get this error

Traceback (most recent call last):
  File "/home/ibiam/yash/starchart/StarChart.py", line 1129, in timer1_cb
    self.plotchart()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1664, in plotchart
    self.plot_whole_sky()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1779, in plot_whole_sky
    self.plot_all_planets()
  File "/home/ibiam/yash/starchart/StarChart.py", line 2194, in plot_all_planets
    self.location.plot_cross()
  File "/home/ibiam/yash/starchart/StarChart.py", line 1054, in plot_cross
    self.context.gc.set_source_rgb(self.colors[4].red, self.colors[4].green, self.colors[4].blue)
KeyError: 4

Steps to reproduce

  • Open activity
  • Click on button before help
  • Select Stars by Constellation in first combo box, select Aquila in second and any in third

It'll also be great if there's a tooltip for the Fullscreen button that says Fullscreen.

@quozl, i also get this error

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/dbus/connection.py", line 230, in maybe_handle_message
    self._handler(*args, **kwargs)
  File "/usr/lib/python2.7/sugar3/presence/connectionmanager.py", line 91, in __status_changed_cb
    self._connections_per_account[account_path].connected = False
KeyError: dbus.ObjectPath('/org/freedesktop/Telepathy/Account/salut/local_xmpp/account0')

What do you think?

@chimosky
Copy link
Member

@quozl my clone was backwards, updated and activity now works fine.

@quozl
Copy link
Contributor

quozl commented Jul 4, 2018

Tested eeacdb3;

  • the constellation lines are significantly thicker; please return their thickness to what it was before the port,
  • grey objects are being drawn in white,
  • a long redraw occurs when stop button is pressed; this must be avoided,
  • 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,

Also a traceback;

Traceback (most recent call last):
  File "/usr/share/sugar/activities/StarChart.activity/StarChart.py", line 1121, in area_expose_cb
    self.plotchart()
  File "/usr/share/sugar/activities/StarChart.activity/StarChart.py", line 1663, in plotchart
    self.plot_whole_sky()
  File "/usr/share/sugar/activities/StarChart.activity/StarChart.py", line 1778, in plot_whole_sky
    self.plot_all_planets()
  File "/usr/share/sugar/activities/StarChart.activity/StarChart.py", line 2193, in plot_all_planets
    self.location.plot_cross()
  File "/usr/share/sugar/activities/StarChart.activity/StarChart.py", line 1053, in plot_cross
    self.context.gc.set_source_rgb(self.colors[4].red, self.colors[4].green, self.colors[4].blue)
AttributeError: Location instance has no attribute 'colors'

@chimosky
Copy link
Member

chimosky commented Jul 6, 2018

@quozl i encountered this error, try cloning again before testing.

@quozl
Copy link
Contributor

quozl commented Jul 7, 2018

It has to work without cloning again, thanks. You'll have to find what is wrong.

@yell0wfl4sh
Copy link
Author

yell0wfl4sh commented Jul 8, 2018

I don't understand the reason of it happening, these errors are not present in mine version except the delay one.

@quozl
Copy link
Contributor

quozl commented Jul 13, 2018

What's a dealy one? Keep debugging; use pdb or other tools, find out why it does not work.

@chimosky
Copy link
Member

@yashagrawal3 click on the button before the last, change Object type: to Deep-sky Objects and the next box Andromeda Galaxy(M031)
Noticed Items in combobox refuse to come up, only available item is the one selected.

@yell0wfl4sh
Copy link
Author

Fixed the Traceback @quozl pasted above. I am still not able to figure out the reason of delay in redraw and long redraw after the close button is pressed. Collapsing toolbar also seems to be a hard task. Any leads will be helpful.

@quozl
Copy link
Contributor

quozl commented Jul 23, 2018

Thanks. Reviewed to 3dbb047. Tested on Ubuntu 16.04 with Sugar 0.112.

In d99c7c7 your commit message didn't explain the problem and how you solved it, but it looks like you copied code from somewhere else. Would it be more appropriate to make the colors array global rather than a class variable?

In c804ea0 your commit message didn't explain the problem and how you solved it, but it looks like you moved the stroke call up, causing the set_line_width and set_source_rgb calls to be ineffective? I'm puzzled.

In 3dbb047 you added a lot of code that could have been factored out into a function to convert a Gdk.Color into RGB.

Investigated. The reasons for the delay in redraw and long redraw on close are;

  • this program uses GTK+ widgets in global variables, which is a style different to what the GTK+ and PyGobject programs usually deploy,
  • the area_expose_cb function improperly declares the cairo context argument as an event (argument names haven't been ported from GTK+ 2 yet), and ignores the context, instead generating a new context from the Gdk.Window,
  • therefore the redraw is not constrained to the dirty area described by the draw event context; the entire area is redrawn, instead of only the dirty area, causing slow drawing,
  • also therefore GTK+ 3's caching of surfaces is bypassed, causing slow drawing,
  • also therefore GTK+ 3's flushing of surface paint operations is bypassed; the drawing operations stick in the GDK queue waiting for the next event of interest to cause a flush; causing the delay,
  • clicking Stop or pressing ctrl+q causes a draw event,

To demonstrate this, add this code temporarily to the end of StarChart.py;

    def close(self, **kwargs):
        # disconnect the draw function to avoid a long delay during close
        self.chart.disconnect_by_func(self.chart.area_expose_cb)
        activity.Activity.close(self, **kwargs)

However, this is a hack, and not an acceptable solution. The area_expose_cb function should be;

  • renamed to a draw callback,
  • use the correct argument names from the GTK+ documentation,
  • not use the Gdk.Window for drawing, but instead the Gtk.DrawingArea ChartDisplay via the context (cr),
  • check the geometry of the context (cr) and not execute major drawing operations for areas outside the geometry.

Also, it would be good to get away from using global variables; while there was once a need to do so on a particularly old version of GTK+ 2, it should not be needed with GTK+ 3.

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.

3 participants